Fix duplicate-name user search results

[wesbillman]

Summary

• preserve distinct people with the same display name in mention autocomplete by keeping pubkey as identity
• paginate the selected-people agent access search in 50-result pages, matching add-member completeness
• keep duplicate names distinguishable with existing NIP-05 or truncated-pubkey secondary labels

Testing

• cd desktop && pnpm build:e2e
• cd desktop && pnpm exec playwright test tests/e2e/mentions.spec.ts tests/e2e/agent-access-warning.spec.ts --project=smoke --grep "same-name"
• pre-push: desktop check, TypeScript, and all 5,278 desktop JS tests

[wesbillman]

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Reviewed exact head 9c0eed1234f904d121adce1842b215820d3a0e83 against base 4e3c9e619c93dd26677b392ad1f8cf0d12c8f855.

Two blocking correctness gaps remain:

1. Preserving duplicate rows does not preserve identity after selection. insertMention still records Map<displayName, pubkey> at desktop/src/features/messages/lib/useMentions.ts:607-617. Select two different people named Will and the second Map.set("Will", ...) overwrites the first; extractMentionPubkeys then applies that surviving pubkey to every matching @Will occurrence (desktop/src/features/messages/lib/extractMentionPubkeys.ts:52-88). Deleting the second visible token can therefore leave the first token notifying the second person. Draft/edit restoration has the same name-keyed shape. The new E2E selects only one duplicate in isolation, so it misses the broken lifecycle. Keep occurrence-bound identity (or insert stable disambiguated labels) and cover select-both, delete-one, send, draft restore, and edit across duplicate person/member/agent/persona labels.

2. Selected-people pagination dead-ends when client filtering leaves no scrollable first page. RespondToField.tsx:136-143 removes already selected and archived users, but the only fetchNextPage trigger is attached to the results scroll container, which renders only when filtered results are nonempty (RespondToField.tsx:417-448; features/profile/hooks.ts:481-501). If the raw first 50 matches are filtered out, or leave too few rows to overflow, nextCursor exists but page 2 is unreachable and a valid later person cannot be selected. Prefetch until the visible list can scroll or the query is exhausted (or provide an explicit load-more path), and test both an entirely filtered first page and a non-overflowing first page.

CI was still in progress when this review was submitted; these defects are source-traced and independently reproduced, not CI-derived.

[wpfleger96]

🤖 Paul, an automated reviewer, commenting via Will's GitHub account. Consolidated feedback from three independent passes (two source reviews, one live E2E run) at head 484b1651e619b829f10548feda8e1acf5ff6908e.

Both source reviews independently reproduced the same two blockers Carl already left above, so I'm confirming rather than restating them in full:

1. Same-name selection still collapses identity after the isolated happy path. insertMention keys the mention map by display name (desktop/src/features/messages/lib/useMentions.ts), so selecting a second distinct "Will" overwrites the first, and extractMentionPubkeys resolves every visible @Will token to the survivor. The new E2E tests select each duplicate in separate tests, so they can't expose the collision. Draft/edit restoration shares the name-keyed shape.

2. Selected-people pagination can dead-end. RespondToField filters selected/archived users from each raw page, but the only fetchNextPage trigger is onScroll on a container that renders only when filtered results are nonempty. A fully filtered or non-overflowing first page makes page 2 unreachable even when nextCursor exists — and the allowlist filter grows with every selection. Prefetch until the list can scroll or the query is exhausted, or add an explicit load-more.

Live E2E note: a 51-person adversarial run of the selected-people flow passed — page-2 fetch fired on scroll and respondToAllowlist preserved both same-name pubkeys. That path stores pubkeys directly, so it doesn't refute either blocker: 1 lives in the mention-map lifecycle, and 2 requires a filtered-empty/non-overflowing first page that a scroll can't reach.

Everything else checked out clean: removal of coalesceAutocompleteCandidatesByKey/globalSearchIdentityKey leaves no dangling references, the infinite-query usage matches the PersonaShareRecipients/MembersSidebar pattern, and the same-name coalescing unit test is correct.

[jedwards27]

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: PR base ref 4e3c9e619c93dd26677b392ad1f8cf0d12c8f855 through exact head 484b1651e619b829f10548feda8e1acf5ff6908e
Risk: high — this changes messaging recipient identity and agent-access authorization selection.

Three blocking correctness/trust gaps remain:

1. Selecting two different people with the same display name in one message drops the first identity. desktop/src/features/messages/lib/useMentions.ts:84,607-616 still stores selections as Map<displayName, pubkey>, so the second Will overwrites the first. extractMentionPubkeys.ts:30,34-53,81-84 can consequently emit only the surviving pubkey. An adversarial E2E selected both duplicate-name rows and sent one composition: expected both pubkeys, observed only the second. The shipped tests select each duplicate only in isolation, so they do not guard the failing lifecycle. Preserve identity by occurrence (or another structure that can hold colliding display names), then add a regression selecting both same-name people in one composition and asserting both outgoing p tags.

2. The “Selected people” access picker can render two distinct pubkeys with identical visible and accessible identity. RespondToField.tsx:62-77,422-446 uses display name plus NIP-05 and omits the pubkey whenever NIP-05 exists. Two profiles claiming Will / will@example.com therefore expose identical buttons even though granting access affects files, accounts, and connected tools. The NIP-05 is copied from self-authored kind-0 JSON without verification state (desktop/src-tauri/src/nostr_convert/user_search.rs:11-25), so it is not a unique or trustworthy discriminator. The new E2E covers only duplicate names without NIP-05 (agent-access-warning.spec.ts:131-174). Collision-detect the rendered tuple and include a pubkey-derived discriminator in both the row and accessible name; make the full key keyboard-accessible in line with shared/ui/PubKey.tsx:18-29. Test two pubkeys sharing both display name and NIP-05, keyboard-select each, and assert the exact persisted pubkey.

3. Selected-people pagination can dead-end after client filtering. RespondToField.tsx:136-145 filters already-selected and archived people after each server page, while the only next-page trigger is onScroll on the results container (RespondToField.tsx:417-448; features/profile/hooks.ts:481-501). If page 1 is fully filtered, the container is not rendered; if too few rows remain to overflow, no scroll event reaches the threshold. A valid person on page 2 is then unreachable despite hasNextPage. Prefetch until the visible list can scroll or the query is exhausted, or provide an explicit load-more action; cover both a fully filtered first page and a non-overflowing first page.

Validation at exact clean head:

• pnpm build:e2e: PASS.
• Focused same-name Playwright cases: 3/3 PASS.
• just desktop-ci: PASS.
• Mutation removing pubkey-based duplicate preservation made the new single-selection tests fail, confirming those tests protect duplicate-row visibility but not the select-both lifecycle.
• Source traced search/coalescing through access mutation payloads; no additional confirmed tenancy/cache or stale-response contamination in the inspected paths.
• GitHub CI at submission: macOS build and Desktop integration green; Desktop Core and smoke shards 1/2/4 still running.

Manual/native evidence: no native capture; these blockers are renderer-contained and were established by executable E2E/source behavior. Residual risk: narrow-layout/theme behavior was not separately captured, and pending CI cannot resolve these untested identity/access paths.

[jedwards27]

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: 4e3c9e619c93dd26677b392ad1f8cf0d12c8f855..206e6074407ebe47cebd69220a5d615afc98b4a3 (exact head 206e6074407ebe47cebd69220a5d615afc98b4a3)
Risk: high — recipient identity and agent-access identity must remain exact when display metadata collides.

Behavior/contracts traced: search pagination/filtering, duplicate-name rendering and accessible disambiguation, keyboard access selection/persistence, mention selection state, and outgoing pubkey extraction.

Blocking finding

• Two distinct same-name recipients still cannot be preserved in one composition. desktop/src/features/messages/lib/useMentions.ts:84 stores selections in Map<displayName, pubkey>; useMentions.ts:607-616 sets by display name, so selecting a second Will overwrites the first; useMentions.ts:782-789 passes the collapsed map into extraction. A clean exact-head adversarial Playwright probe selected pubkeys 111…111 and 222…222 as two @Will mentions, expected both outgoing pubkeys, and received only 222…222. The shipped cases at desktop/tests/e2e/mentions.spec.ts:690-735 select either duplicate alone and therefore miss the failure. Represent mention identity by occurrence/pubkey rather than display-name key, and add an E2E selecting both identities in one composition, deleting either occurrence, and asserting the exact remaining/sent pubkeys.

Resolved prior findings: the refreshed head now handles filtered/non-scrollable pagination and always exposes pubkey-based visible/accessibility disambiguation in the agent-access picker; the hostile identical-name/NIP-05 keyboard persistence case passes.

Validation at matching clean HEAD: pnpm build:e2e PASS; focused shipped Playwright 5/5 PASS; access collision/pagination 3/3 PASS; same-name single-selection mentions 2/2 PASS; the temporary select-both probe failed causally as described and was removed. One combined Playwright attempt lost its local server and was discarded as infra-invalid.

Manual/native evidence: not run; both the resolved behaviors and remaining failure are renderer-contained and executable/source-deterministic.

Residual risk: Desktop Core and smoke shards were still pending at lane refresh. Their completion cannot repair the reproduced untested recipient-loss path.

[jedwards27]

Verdict: REQUEST CHANGES

Reviewed: 4e3c9e619c93dd26677b392ad1f8cf0d12c8f855..206e6074407ebe47cebd69220a5d615afc98b4a3 (exact head 206e6074407ebe47cebd69220a5d615afc98b4a3)

Risk: high — this changes messaging recipient identity and selected-people access behavior.

The new head resolves the previous pagination and access-picker ambiguity: filtered non-scrollable results continue fetching, each access result visibly exposes its pubkey, and the Add button's accessible name includes the full key. The hostile same-name/same-NIP-05 keyboard case correctly selects the intended pubkey.

Blocking finding

• desktop/src/features/messages/lib/useMentions.ts:84,607-616,782-789 still keys selected mentions by displayName. Selecting pubkey 111…111 as @Will and then pubkey 222…222 as another @Will overwrites the first map entry. The composer can display both tokens, but the outgoing mention list contains only 222…222; the first intended recipient is silently lost. This violates the PR's core identity-preservation contract. The added cases in desktop/tests/e2e/mentions.spec.ts:691-735 select either identity separately, so they do not cover the failing two-recipient composition.

Please preserve mention identity by occurrence/pubkey rather than display-name key, then add an E2E that selects both same-name identities in one composition, verifies deletion of either occurrence leaves the correct identity, and asserts both exact outgoing pubkeys when both remain.

Validation at matching clean HEAD:

• cd desktop && pnpm build:e2e — PASS independently.
• Focused shipped same-name and filtered/non-scrollable pagination Playwright coverage — PASS (5/5 in one run; independently repeated as 3/3 access/pagination and 2/2 mention cases).
• Temporary adversarial Playwright selecting both same-name rows — FAIL as expected: expected [111…111, 222…222], received [222…222]; probe removed and clean head restored.
• One earlier combined Playwright attempt lost its local HTTP server after a timeout; treated as infrastructure-invalid and replaced by clean passing focused runs.

Manual/native evidence: not run. The fixed access semantics and remaining blocker are renderer-contained and established directly by source plus executable E2E evidence.

Residual risk: CI was still in progress at review time (Desktop Core/smoke jobs pending), but green CI cannot cover the missing multi-recipient case.

— :bot: Jude’s code review agent