Merge pull request 'fix(account): dedupe colliding identities on merge, journaling to the dossier' (#167) from feature/merge-email-dedupe into development

backend/internal/account/retention.go

@@ -21,9 +21,11 @@ import (
 const RetentionTTL = 2 * 365 * 24 * time.Hour
 
 // Reasons recorded on a retained_identities row: what detached the credential from its
-// account. The row is written just before the live identities row is removed, preserving
+// account (unlink / email change / account deletion here; an account merge that drops a
-// the legal dossier (which email/vk/tg was linked, and when) even as the identity frees
+// same-kind colliding identity writes reason "merge" from the accountmerge package). The
-// for reuse. See docs/ARCHITECTURE.md §9.1.
+// row is written just before the live identities row is removed, preserving the legal
+// dossier (which email/vk/tg was linked, and when) even as the identity frees for reuse.
+// See docs/ARCHITECTURE.md §9.1.
 const (
 	retainUnlink = "unlink"
 	retainChange = "change"

backend/internal/accountmerge/merge.go

@@ -27,6 +27,11 @@ import (
 // without taking a dependency on the game package.
 const statusActive = "active"
 
+// retainReasonMerge is the retained_identities.reason for a credential dropped by a merge
+// collision (both accounts held the same kind). It mirrors the account package's retain
+// reasons, kept local to avoid importing that package's unexported constants.
+const retainReasonMerge = "merge"
+
 // Friendship statuses, highest precedence first, mirroring internal/social.
 const (
 	friendAccepted = "accepted"
@@ -75,6 +80,9 @@ func (m *Merger) Merge(ctx context.Context, primary, secondary uuid.UUID) error
 		if err := mergeAccountFields(ctx, tx, primary, secondary, now); err != nil {
 			return err
 		}
+		if err := dedupeIdentities(ctx, tx, primary, secondary); err != nil {
+			return err
+		}
 		if err := reassignColumn(ctx, tx, table.Identities, table.Identities.AccountID, primary, secondary); err != nil {
 			return fmt.Errorf("accountmerge: identities: %w", err)
 		}
@@ -300,6 +308,77 @@ func reassignColumn(ctx context.Context, tx *sql.Tx, tbl postgres.Table, col pos
 	return err
 }
 
+// dedupeIdentities resolves a same-kind identity collision before the blanket identity
+// reassign: when both accounts already hold an identity of the same kind (e.g. each has a
+// confirmed email — reachable when two email-bearing accounts merge), the primary keeps
+// its own and the secondary's is journaled to retained_identities (reason=merge) and
+// removed. Without this the blanket reassign would leave the survivor with two identities
+// of one kind (there is no per-account-kind unique on identities), which the profile and
+// the retention dossier both treat as singular. Non-colliding identities are untouched and
+// move with the blanket reassign.
+func dedupeIdentities(ctx context.Context, tx *sql.Tx, primary, secondary uuid.UUID) error {
+	var prows []model.Identities
+	if err := postgres.SELECT(table.Identities.Kind).
+		FROM(table.Identities).
+		WHERE(table.Identities.AccountID.EQ(postgres.UUID(primary))).
+		QueryContext(ctx, tx, &prows); err != nil && !errors.Is(err, qrm.ErrNoRows) {
+		return fmt.Errorf("accountmerge: primary identity kinds: %w", err)
+	}
+	occupied := make(map[string]struct{}, len(prows))
+	for _, r := range prows {
+		occupied[r.Kind] = struct{}{}
+	}
+	if len(occupied) == 0 {
+		return nil
+	}
+	var srows []model.Identities
+	if err := postgres.SELECT(
+		table.Identities.Kind, table.Identities.ExternalID,
+		table.Identities.Confirmed, table.Identities.CreatedAt,
+	).FROM(table.Identities).
+		WHERE(table.Identities.AccountID.EQ(postgres.UUID(secondary))).
+		QueryContext(ctx, tx, &srows); err != nil && !errors.Is(err, qrm.ErrNoRows) {
+		return fmt.Errorf("accountmerge: secondary identities: %w", err)
+	}
+	for _, s := range srows {
+		if _, dup := occupied[s.Kind]; !dup {
+			continue
+		}
+		if err := retainMergedIdentity(ctx, tx, secondary, s); err != nil {
+			return err
+		}
+		del := table.Identities.DELETE().WHERE(
+			table.Identities.AccountID.EQ(postgres.UUID(secondary)).
+				AND(table.Identities.Kind.EQ(postgres.String(s.Kind))).
+				AND(table.Identities.ExternalID.EQ(postgres.String(s.ExternalID))),
+		)
+		if _, err := del.ExecContext(ctx, tx); err != nil {
+			return fmt.Errorf("accountmerge: drop colliding %s identity: %w", s.Kind, err)
+		}
+	}
+	return nil
+}
+
+// retainMergedIdentity appends a retained_identities row for a secondary identity dropped
+// by a merge collision (reason=merge), preserving it in the legal dossier. It mirrors
+// account.retainIdentityTx, which is unexported; detached_at falls to the column default.
+func retainMergedIdentity(ctx context.Context, tx *sql.Tx, accountID uuid.UUID, id model.Identities) error {
+	rid, err := uuid.NewV7()
+	if err != nil {
+		return fmt.Errorf("accountmerge: new retained id: %w", err)
+	}
+	ins := table.RetainedIdentities.INSERT(
+		table.RetainedIdentities.RetainedID, table.RetainedIdentities.AccountID,
+		table.RetainedIdentities.Kind, table.RetainedIdentities.ExternalID,
+		table.RetainedIdentities.Confirmed, table.RetainedIdentities.LinkedAt,
+		table.RetainedIdentities.Reason,
+	).VALUES(rid, accountID, id.Kind, id.ExternalID, id.Confirmed, id.CreatedAt, retainReasonMerge)
+	if _, err := ins.ExecContext(ctx, tx); err != nil {
+		return fmt.Errorf("accountmerge: retain merged %s identity: %w", id.Kind, err)
+	}
+	return nil
+}
+
 // friendRank ranks a friendship status for dedupe precedence (higher wins).
 func friendRank(status string) int {
 	switch status {

backend/internal/inttest/merge_test.go

@@ -251,6 +251,44 @@ func TestAccountMergeFinishedSharedGameKept(t *testing.T) {
 	}
 }
 
+// TestAccountMergeDedupesEmail keeps the primary's email when both accounts have one and
+// journals the secondary's to the dossier (reason=merge), so the survivor never ends up
+// with two email identities.
+func TestAccountMergeDedupesEmail(t *testing.T) {
+	ctx := context.Background()
+	store := account.NewStore(testDB)
+	merger := accountmerge.NewMerger(testDB)
+
+	primary := provisionAccount(t)
+	secondary := provisionAccount(t)
+	primaryEmail := "keep-" + uuid.NewString() + "@example.com"
+	secondaryEmail := "absorb-" + uuid.NewString() + "@example.com"
+	bindEmailIdentity(t, primary, primaryEmail)
+	bindEmailIdentity(t, secondary, secondaryEmail)
+
+	if err := merger.Merge(ctx, primary, secondary); err != nil {
+		t.Fatalf("merge: %v", err)
+	}
+
+	// The primary keeps its own email; the secondary's is gone from the live identities.
+	if owner, ok, _ := store.AccountIDByIdentity(ctx, account.KindEmail, primaryEmail); !ok || owner != primary {
+		t.Errorf("primary email owner = %s ok=%v, want primary %s", owner, ok, primary)
+	}
+	if _, ok, _ := store.AccountIDByIdentity(ctx, account.KindEmail, secondaryEmail); ok {
+		t.Error("the secondary's email must be removed (no duplicate email on the survivor)")
+	}
+	// The absorbed email stays in the legal dossier, tagged reason=merge.
+	var reason string
+	if err := testDB.QueryRowContext(ctx,
+		`SELECT reason FROM backend.retained_identities WHERE account_id=$1 AND kind='email' AND external_id=$2`,
+		secondary, secondaryEmail).Scan(&reason); err != nil {
+		t.Fatalf("retained email row: %v", err)
+	}
+	if reason != "merge" {
+		t.Errorf("retained reason = %q, want merge", reason)
+	}
+}
+
 // TestAccountLinkFreeEmail binds a free email and promotes a guest to durable.
 func TestAccountLinkFreeEmail(t *testing.T) {
 	ctx := context.Background()

backend/internal/postgres/migrations/00008_retained_identities_merge_reason.sql

@@ -0,0 +1,22 @@
+-- Admit the 'merge' reason into retained_identities. An account merge folds a secondary
+-- account into a primary; when both hold an identity of the same kind (e.g. each has a
+-- confirmed email), the survivor keeps its own and the secondary's is journaled to the
+-- legal dossier and removed — so the survivor never ends up with two identities of one
+-- kind, and the absorbed credential is still retained. That journal row carries reason
+-- 'merge', alongside the existing unlink / change / delete.
+--
+-- Expand-contract: the Up only WIDENS the allowed reason set, so an older backend image
+-- (which writes only unlink/change/delete) still satisfies the constraint — a rollback
+-- stays DB-safe. The Down narrows it again and would reject pre-existing 'merge' rows, so
+-- it is a dev-only convenience, not a production rollback path (image rollback runs old
+-- code against this schema, not the Down migration).
+
+-- +goose Up
+ALTER TABLE backend.retained_identities DROP CONSTRAINT retained_identities_reason_chk;
+ALTER TABLE backend.retained_identities ADD CONSTRAINT retained_identities_reason_chk
+    CHECK ((reason = ANY (ARRAY['unlink'::text, 'change'::text, 'delete'::text, 'merge'::text])));
+
+-- +goose Down
+ALTER TABLE backend.retained_identities DROP CONSTRAINT retained_identities_reason_chk;
+ALTER TABLE backend.retained_identities ADD CONSTRAINT retained_identities_reason_chk
+    CHECK ((reason = ANY (ARRAY['unlink'::text, 'change'::text, 'delete'::text])));

docs/ARCHITECTURE.md

@@ -292,8 +292,11 @@ arrive from a platform rather than completing a mandatory registration).
   invitations / friend-codes / drafts / pending-codes. **Chat, feedback and complaints are
   kept.** The orchestration a layer up resigns the account's active games (so opponents are
   not stranded), **drops its all-robot games** (no human opponent; children cascade), and
-  revokes its sessions. Every credential detachment — **unlink, email change and delete** —
+  revokes its sessions. Every credential detachment — **unlink, email change, delete and a
-  writes a `retained_identities` row (`reason`), so the dossier keeps the full timeline.
+  merge collision** — writes a `retained_identities` row (`reason`), so the dossier keeps the
+  full timeline. (A merge that would otherwise leave the survivor with two identities of one
+  kind — e.g. each account held a confirmed email — keeps the primary's and journals the
+  secondary's with `reason=merge` before dropping it.)
   Step-up is a mailed code (`purpose=delete`, no deeplink — a stray click must not delete;
   `ConfirmByToken` refuses a delete token) for an email account, else a typed phrase.
   `last_login_at` / `last_login_ip` are stamped on the cold-load profile fetch (throttled