From 3c39b7d2a0a0692dab4a915e4d9c2c8691775385 Mon Sep 17 00:00:00 2001 From: viettranx Date: Fri, 17 Apr 2026 19:04:37 +0700 Subject: [PATCH] fix(contacts): merge contact into existing tenant user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - user_search: return tenant_user UUID in new "uuid" field so callers forwarding a tenant_user foreign key receive the primary key instead of the user_id string (root cause of HTTP 400 "invalid JSON" on /v1/contacts/merge when linking to an existing tenant user) - use-user-picker: add opt-in valueMode prop ("user_id" default | "uuid") so only merge-contacts-dialog commits UUID; other pickers (tenant add-user, MCP/CLI credentials) keep returning user_id string - pg.MigrateUserDataOnMerge: split tClauseIns/tClauseDel with correct placeholder offsets (N+2 for INSERT/UPDATE, N+1 for DELETE) — reusing one clause caused "no parameter \$N+2" errors that silently skipped every cleanup DELETE, leaving orphan per-user rows after a merge --- internal/http/user_search.go | 21 +++++++----- internal/store/pg/agents_context.go | 34 ++++++++++++------- .../shared/user-picker-combobox.tsx | 8 ++++- ui/web/src/hooks/use-user-picker.ts | 23 ++++++++++--- .../pages/contacts/merge-contacts-dialog.tsx | 1 + 5 files changed, 62 insertions(+), 25 deletions(-) diff --git a/internal/http/user_search.go b/internal/http/user_search.go index 1ae09cdf..beb8a470 100644 --- a/internal/http/user_search.go +++ b/internal/http/user_search.go @@ -12,15 +12,19 @@ import ( ) // UserSearchResult is a unified result from contacts + tenant_users. +// ID is the user_id string (human-facing identifier). UUID is the tenant_user +// primary key (only populated when Source == "tenant_user"); callers that need +// to reference a tenant_user by foreign key (e.g. contact merge) must use UUID. type UserSearchResult struct { - ID string `json:"id"` - DisplayName *string `json:"display_name,omitempty"` - Username *string `json:"username,omitempty"` - Source string `json:"source"` // "contact" or "tenant_user" - ChannelType *string `json:"channel_type,omitempty"` - PeerKind *string `json:"peer_kind,omitempty"` - MergedTenantUserID *string `json:"merged_tenant_user_id,omitempty"` - Role *string `json:"role,omitempty"` + ID string `json:"id"` + UUID string `json:"uuid,omitempty"` + DisplayName *string `json:"display_name,omitempty"` + Username *string `json:"username,omitempty"` + Source string `json:"source"` // "contact" or "tenant_user" + ChannelType *string `json:"channel_type,omitempty"` + PeerKind *string `json:"peer_kind,omitempty"` + MergedTenantUserID *string `json:"merged_tenant_user_id,omitempty"` + Role *string `json:"role,omitempty"` } // handleSearchUsers returns unified results from channel_contacts + tenant_users. @@ -91,6 +95,7 @@ func (h *ChannelInstancesHandler) handleSearchUsers(w http.ResponseWriter, r *ht role := u.Role results = append(results, UserSearchResult{ ID: u.UserID, + UUID: u.ID.String(), DisplayName: u.DisplayName, Source: "tenant_user", Role: &role, diff --git a/internal/store/pg/agents_context.go b/internal/store/pg/agents_context.go index 2f6c62d7..113338ef 100644 --- a/internal/store/pg/agents_context.go +++ b/internal/store/pg/agents_context.go @@ -126,7 +126,16 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs [] if len(oldUserIDs) == 0 { return nil } - tClause, tArgs, _, err := scopeClause(ctx, len(oldUserIDs)+2) + // INSERT / UPDATE queries pass args in order [oldIDs..., newUserID, tArgs...], + // so tenant-scope placeholders start at position N+2. + tClauseIns, tArgs, _, err := scopeClause(ctx, len(oldUserIDs)+2) + if err != nil { + return err + } + // DELETE queries drop the newUserID placeholder and pass [oldIDs..., tArgs...], + // so tenant-scope placeholders must start at N+1. Reusing tClauseIns here + // produces "there is no parameter $N+2" errors that silently corrupt cleanup. + tClauseDel, _, _, err := scopeClause(ctx, len(oldUserIDs)+1) if err != nil { return err } @@ -143,8 +152,8 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs [] baseArgs = append(baseArgs, tArgs...) // Args for delete (no newUserID param needed). - delArgs := make([]any, len(oldUserIDs)) - copy(delArgs, baseArgs[:len(oldUserIDs)]) + delArgs := make([]any, 0, len(oldUserIDs)+len(tArgs)) + delArgs = append(delArgs, baseArgs[:len(oldUserIDs)]...) delArgs = append(delArgs, tArgs...) // Helper: migrate + delete for one table. DO NOTHING on conflict — @@ -163,8 +172,8 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs [] fmt.Sprintf(`INSERT INTO user_context_files (id, agent_id, user_id, file_name, content, updated_at, tenant_id) SELECT gen_random_uuid(), agent_id, %s, file_name, content, updated_at, tenant_id FROM user_context_files WHERE user_id IN (%s)%s - ON CONFLICT (agent_id, user_id, file_name) DO NOTHING`, newP, inClause, tClause), - fmt.Sprintf(`DELETE FROM user_context_files WHERE user_id IN (%s)%s`, inClause, tClause), + ON CONFLICT (agent_id, user_id, file_name) DO NOTHING`, newP, inClause, tClauseIns), + fmt.Sprintf(`DELETE FROM user_context_files WHERE user_id IN (%s)%s`, inClause, tClauseDel), ) // 2. user_agent_overrides: UNIQUE(agent_id, user_id) @@ -172,8 +181,8 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs [] fmt.Sprintf(`INSERT INTO user_agent_overrides (id, agent_id, user_id, provider, model, settings, created_at, updated_at, tenant_id) SELECT gen_random_uuid(), agent_id, %s, provider, model, settings, created_at, updated_at, tenant_id FROM user_agent_overrides WHERE user_id IN (%s)%s - ON CONFLICT (agent_id, user_id) DO NOTHING`, newP, inClause, tClause), - fmt.Sprintf(`DELETE FROM user_agent_overrides WHERE user_id IN (%s)%s`, inClause, tClause), + ON CONFLICT (agent_id, user_id) DO NOTHING`, newP, inClause, tClauseIns), + fmt.Sprintf(`DELETE FROM user_agent_overrides WHERE user_id IN (%s)%s`, inClause, tClauseDel), ) // 3. user_agent_profiles: PK(agent_id, user_id) @@ -181,8 +190,8 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs [] fmt.Sprintf(`INSERT INTO user_agent_profiles (agent_id, user_id, workspace, first_seen_at, last_seen_at, metadata, tenant_id) SELECT agent_id, %s, workspace, first_seen_at, last_seen_at, metadata, tenant_id FROM user_agent_profiles WHERE user_id IN (%s)%s - ON CONFLICT (agent_id, user_id) DO NOTHING`, newP, inClause, tClause), - fmt.Sprintf(`DELETE FROM user_agent_profiles WHERE user_id IN (%s)%s`, inClause, tClause), + ON CONFLICT (agent_id, user_id) DO NOTHING`, newP, inClause, tClauseIns), + fmt.Sprintf(`DELETE FROM user_agent_profiles WHERE user_id IN (%s)%s`, inClause, tClauseDel), ) // 4. memory_documents: UNIQUE(agent_id, COALESCE(user_id,''), path) @@ -190,14 +199,15 @@ func (s *PGAgentStore) MigrateUserDataOnMerge(ctx context.Context, oldUserIDs [] fmt.Sprintf(`INSERT INTO memory_documents (id, agent_id, user_id, path, content, hash, updated_at, created_at, tenant_id) SELECT gen_random_uuid(), agent_id, %s, path, content, hash, updated_at, created_at, tenant_id FROM memory_documents WHERE user_id IN (%s)%s - ON CONFLICT (agent_id, COALESCE(user_id,''), path) DO NOTHING`, newP, inClause, tClause), - fmt.Sprintf(`DELETE FROM memory_documents WHERE user_id IN (%s)%s`, inClause, tClause), + ON CONFLICT (agent_id, COALESCE(user_id,''), path) DO NOTHING`, newP, inClause, tClauseIns), + fmt.Sprintf(`DELETE FROM memory_documents WHERE user_id IN (%s)%s`, inClause, tClauseDel), ) // 5. memory_chunks: FK on document_id — cascade from memory_documents delete handles this. // But orphan chunks (where document was already migrated) need cleanup. // Simply re-point remaining chunks whose document still has old user_id. - repoint := fmt.Sprintf(`UPDATE memory_chunks SET user_id = %s WHERE user_id IN (%s)%s`, newP, inClause, tClause) + // Uses INSERT-style arg layout (newUserID at N+1, tenant at N+2). + repoint := fmt.Sprintf(`UPDATE memory_chunks SET user_id = %s WHERE user_id IN (%s)%s`, newP, inClause, tClauseIns) if _, err := s.db.ExecContext(ctx, repoint, baseArgs...); err != nil { slog.Warn("merge.migrate_chunks", "error", err) } diff --git a/ui/web/src/components/shared/user-picker-combobox.tsx b/ui/web/src/components/shared/user-picker-combobox.tsx index 729ad7ce..7d07d436 100644 --- a/ui/web/src/components/shared/user-picker-combobox.tsx +++ b/ui/web/src/components/shared/user-picker-combobox.tsx @@ -13,6 +13,11 @@ interface UserPickerComboboxProps { /** Filter by source: "contact" | "tenant_user" | undefined (both). * Use "tenant_user" for merge dialogs and tenant user pickers. */ source?: "contact" | "tenant_user"; + /** Committed value shape. "user_id" (default) returns the human-facing user_id + * string; "uuid" returns the tenant_user primary key UUID and is only useful + * when the consumer forwards the value to a backend expecting a tenant_user + * foreign key (e.g. contact merge's `tenant_user_id`). Requires `source="tenant_user"`. */ + valueMode?: "user_id" | "uuid"; /** Allow typing custom values not in the list. Default true. */ allowCustom?: boolean; /** Render dropdown into a portal container (useful inside dialogs). */ @@ -38,10 +43,11 @@ export function UserPickerCombobox({ className, peerKind, source, + valueMode, allowCustom = true, portalContainer, }: UserPickerComboboxProps) { - const { options } = useUserPicker(value, peerKind, source); + const { options } = useUserPicker(value, peerKind, source, valueMode); return ( results.map((r) => { const parts: string[] = []; @@ -61,9 +75,10 @@ export function useUserPicker(search: string, peerKind?: string, source?: "conta if (r.source === "contact" && r.channel_type) parts.push(`[${r.channel_type}]`); if (r.source === "tenant_user") parts.push("[tenant]"); if (r.merged_tenant_user_id) parts.push(`→ ${r.merged_tenant_user_id}`); - return { value: r.id, label: parts.join(" ") }; + const value = valueMode === "uuid" && r.source === "tenant_user" && r.uuid ? r.uuid : r.id; + return { value, label: parts.join(" ") }; }), - [results], + [results, valueMode], ); return { results, options, loading: isLoading }; diff --git a/ui/web/src/pages/contacts/merge-contacts-dialog.tsx b/ui/web/src/pages/contacts/merge-contacts-dialog.tsx index 97ed550b..92fe8969 100644 --- a/ui/web/src/pages/contacts/merge-contacts-dialog.tsx +++ b/ui/web/src/pages/contacts/merge-contacts-dialog.tsx @@ -117,6 +117,7 @@ export function MergeContactsDialog({ onChange={setSelectedUserId} placeholder={t("merge.selectUser")} source="tenant_user" + valueMode="uuid" /> )}