Repository navigation
AIDA-2436: Find DM recipients by username - #2324
ngthuydiem wants to merge 3 commits into
Conversation
Match directory usernames and known identity names, and include cached profiles when searching for recipients. Co-authored-by: Sol <aca808ddc61bef05135a9ca77e7b61324d61af5ed367852d6c4f38babf8543af@buzz.block.builderlab.xyz> Signed-off-by: Diem Nguyen <diem@squareup.com>
Co-authored-by: Sol <aca808ddc61bef05135a9ca77e7b61324d61af5ed367852d6c4f38babf8543af@buzz.block.builderlab.xyz> Signed-off-by: Diem Nguyen <diem@squareup.com>
kazemek21
left a comment
There was a problem hiding this comment.
Kyle reviewed the PR and the Nexus review-tracker notes and approved with the non-blocking comment below.
Posted by Codex (gpt-6.1-sol) on behalf of kylez
| ); | ||
| if (query.trim()) | ||
| for (const [pubkey, profile] of profiles) | ||
| if (!known.has(pubkey) && (!profile.nameIsFallback || profile.username)) |
There was a problem hiding this comment.
[P2] Preserve live cached usernames for existing directory rows
If the picker has already browsed someone and their profile then publishes a new username, this guard keeps the old directory row and skips the newer cached profile. A completed browse also prevents another directory search during that visit, so searching the current username can show “No matching people” even though it is already cached. Could we include the current profile aliases for existing keys too, preserving the eligibility metadata, and cover a profile update after the initial browse?
Posted by Codex (gpt-6.1-sol) on behalf of kylez
Preserve directory eligibility while considering the latest cached profile name and username for existing keys. Co-authored-by: Sol <aca808ddc61bef05135a9ca77e7b61324d61af5ed367852d6c4f38babf8543af@buzz.block.builderlab.xyz> Signed-off-by: Diem Nguyen <diem@squareup.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account.
[P2] Remove the internal tracker link from the public PR description. The opening “Why” paragraph links to the company’s internal Linear workspace. This public repository’s description exposes an internal coordination reference; keep the standalone explanation of the recipient-search defect and remove the internal URL. This is a publication-surface finding; it does not imply that credentials or issue contents were disclosed.
Star Lord, Rocket, Mantis and Groot performed source review. No actionable implementation defect was found in username/identity alias matching, cached-profile merging, or recipient selection. Optional: show the matching username/alias as supporting text when it differs from the displayed profile name.
Reviewed head eeb2ffcec9281ece3640e201da2c257529fbc6ba, base d33e5452927750fc0e727d8a86a90cdd0b772d4a (feature merge-base f5c0dd013617d40dd760ea00e6b9e3f1622548e3). Source-only; no tests, builds or app execution performed. Native focus, screen-reader behavior, live-community reproduction and human acceptance remain unverified. The linked issue returned only the generic Linear app shell, so its contents were unavailable. No description attachments were present. One hosted-check snapshot showed CI required successful; Windows native validation and the browser flake-check job were skipped. This is a non-blocking COMMENT review, not approval or merge authorization.
|
🤖 Closing as superseded by #2585, which rebuilt New message recipient matching around verified identity. One small gap remains: unverified people whose kind-0 handle differs from their display name. It can be fixed separately on top of main if needed. |
## Summary - Pasting a colleague's `npub`, copied from their profile, into **New message** or **⌘K** found nobody. Pasting `nostr:npub1…` into **Add people** found nobody too. - Three separate filters each dropped the row: - the relay directory reader treated only lowercase hex as an exact key; - New message's `matching()` kept only key rows typed as lowercase hex; - ⌘K's `rankPeople` kept only name matches, so even lowercase hex was lost. - Now an `npub` (with or without `nostr:`, in lowercase or all caps) or a 64-character hex key in either case finds the person on all three surfaces. In ⌘K they are listed first under People. - After `@` in a message, a complete pasted key now lists only that key's owner, and Space completes it. Before, the mention chooser dropped the row the directory found for the key. - New message also finds a local agent the relay has no profile for by its key. _Left: `main` 8668dbd. Right: this branch. The same script copies Sam Rivera's npub, then pastes it into New message, ⌘K and Add people (with `nostr:` typed first). Before, nobody is found. After, Sam is found every time._ https://github.com/user-attachments/assets/19418239-e4fd-40f7-aa1b-3e3850d76c7d ## Details - New `typedPublicKey()` in `src/shared/identity/public-key.ts` returns the hex key that typed text names exactly: 64 hex characters in either case, or an `npub` with or without `nostr:` (case-insensitive prefix). The text is trimmed first. It returns nothing for anything else, including a key still being typed, an npub with a bad checksum, an npub in mixed case, or an npub whose payload is not 32 bytes (nip19 does not check payload length itself). - One parser, used by every place that reads a typed key: - the relay reader's exact `authors` read (`features/relay/direct-messages.ts`); - New message's `matching()` (`features/direct-messages/usePeople.ts`); - ⌘K's `rankPeople` (`features/search/people-rank.ts`), where an exact key ranks 0; - Add people's member search (`features/channel-members/search.ts`), replacing its inline nip19 decode; - `ChannelMembersDialog`'s local-agent and current-member filters, so `nostr:npub…` also finds a local agent the relay has no profile for, and finds an existing member; - the mention directory's exact-key check (`bundled/mentions/useMentionDirectory.ts`), so a pasted key is an author lookup there too, not an answer from a cached name page; - the mention chooser's ranking, name index and Space rule (`bundled/mentions/mention-ranking.ts`). A complete key matches only its owner, at tier 0, even an owner with no profile name. A profile whose name is the key text does not match it. The mentions contract (`bundled/mentions/README.md`) now defines an exact public key under Terms and uses it in sections 3, 5 and 6. It is version 3, because this reverses a MUST NOT; - New message's local-agent rows (`features/direct-messages/RecipientPicker.tsx`), with the same eligibility checks; - community admin's **Add directly** (`bundled/moderation/CommunityAdmin.tsx`); - the relay staff pickers: Direct actions' **Member** field and Staff's **Public key** (`bundled/relay-staff/DirectActions.tsx`, `Operators.tsx`). `publicKeyInput` is removed. - Before, Add directly and the staff pickers refused `nostr:npub…`. They also read a valid-checksum npub with a 20-byte payload as a 40-character key, which went to the relay in a member or operator write. Now that npub is refused before any request is sent. - A partly typed npub is still an ordinary name search, and in mentions partial keys and partial npubs still match nothing. `matchPerson`, the mention name rule, is unchanged. - No UI or copy changes. One sentence in `docs/buzz-help.md` says where a pasted public key finds someone, now including after `@`. ## Before → after Each spelling ran in its own app instance against both builds. "found, first" means listed first under People. | Pasted text | New message, small dir | New message, large dir | ⌘K | Add people | Mentions, after `@` ¹ | |---|---|---|---|---|---| | `npub1…` | nobody → found | nobody → found | nobody → found, first | found → found | nobody → found | | `nostr:npub1…` | nobody → found | nobody → found | nobody → found, first | nobody → found | nobody → found | | `NPUB1…` | nobody → found | nobody → found | nobody → found, first | nobody → found | nobody → found | | `NOSTR:NPUB1…` | nobody → found | nobody → found | nobody → found, first | nobody → found | nobody → found | | hex, uppercase | found → found | nobody → found | nobody → found, first | found → found | nobody → found | | hex, lowercase | found → found | found → found | nobody → found, first | found → found | nobody → found | | A local agent's key, no relay profile ² | nobody → found | not tested | not tested | nobody → found (`nostr:` tested) | not tested | ¹ From the ranking unit tests and portable fixtures (every spelling), plus desktop component tests in `solo-agents.test.tsx`: a member beside a namesake, someone outside the channel, and the **Mention a member** picker. Not from the in-app probe. "found" means the owner is the only row and Space completes it. ² From component tests, not the probe. New message: hex, `npub1…` and `nostr:npub1…` with a complete directory, and an agent from another community stays absent. Add people: `nostr:npub1…`, from an earlier commit in this PR. ## Tests - 34 new test cases fail without the fix and pass here. They cover `public-key`, the relay reader, `NewMessage`, `people-rank`, `SearchResults`, member search, `ChannelMembersDialog` and `useMentionDirectory`. - The review follow-up adds or rewrites 18 more cases, and each one fails on the code before its commit: - mentions: the rewritten key test in `mention-ranking.test.ts`, 7 portable fixture cases, 2 new Space component tests, and 2 component tests that pinned the old "keys match no choice" rule; - New message: 3 local-agent cases; - admin and staff: Add directly, the Direct actions picker and Staff, one each. - Two partial-npub fixture cases pass before and after, as guards. - The touched Vitest files and the related tests for `mention-ranking.ts` and `RecipientPicker.tsx` pass. Typecheck and lint are green. Browser cases: 0 added, 0 removed. One stale comment in `tests/browser/mentions.spec.mjs` changed. - The recording and the first six table rows come from headless Chromium against the fixture relay. They were not run in the native Tauri app, in WebKit or against a live relay. The `team-mentions` probe (paste a key after `@`, then Space) has not been re-run on this head. ## Merge note This head is based on `main` b1e3f4c. #2298 (ef5a784) has since landed and conflicts in `src/bundled/mentions/mention-ranking.ts`. Its `rankMentions` now calls `labelMatch` directly, not `mentionMatch`, so the rebase must apply the key-owner rule there too. The portable ranking fixtures fail if it doesn't. #2324 was closed without merging, so its `usePeople.ts` conflict no longer applies. ## Not covered - Message search's `from:` operator still accepts only a hex key. Its doc comment mentions npub. That is a natural follow-up use of `typedPublicKey()` and is left out of this PR. - In New message and ⌘K, a profile whose *name* is the pasted npub or hex text still matches it by name. In a small directory, New message's `matching()` keeps that profile at tier 0, and Space can pick it through `exactRecipient`. This predates the PR. Mentions now match a complete key only to its owner. Doing the same in New message's `matching()` and `exactRecipient`, and in ⌘K's `rankPeople`, is a follow-up. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Why
New message can show “No matching people” for a username that global search finds. The recipient hook checked only the display name and omitted session-cached profiles.
What
Find recipients by their profile name, username, or resolved identity name. Include already-known profiles in typed searches, as global search does.
How
Keep raw directory rows until matching so aliases are available when the query or identity names change. Use the same aliases for recipient ranking and exact-name keyboard completion. For existing directory keys, include live cached names as matching-only aliases without replacing directory eligibility metadata. The shared agent-sharing recipient hook gets the matching fix too; each picker keeps its existing eligibility checks.
Risk
This changes client matching and cached suggestions, not relay discovery. A person absent from both the directory response and the local cache still cannot appear. The reporter’s exact relay response remains unverified. Related PR #2322 explicitly excludes the recipient picker.
Testing
Existing New message journeys passed in Chromium and WebKit:
bin/pnpm exec playwright test --config tests/browser/playwright.config.mjs tests/browser/new-message.spec.mjs --project chromium --project webkit --no-deps(12 cases). This exercises fixture integration, not the reporter’s community.Draft: human testing and live validation in the reporter’s community remain outstanding. To try: open New message, type a username whose display name differs, select the person, and confirm the message opens the intended DM.
The full local Vitest run passed 15,880 tests and failed one unchanged media conversion test:
browser-host/media-preparation.integration.test.mjsproduced a 512×512 HEIC image instead of 1536×1024. Running that file alone reproduced it with local ffmpeg 8.0.1. Media code and fixtures are unchanged by this PR. Pre-commit/pre-push checks passed.Regression tests cover directory username matching, a cached profile with no directory result, resolved identity names, and exact-name keyboard completion. No browser cases added or removed.
The share-card browser fixture now supplies the profile-cache interface used by the recipient hook. All eight share-card layout cases passed in Chromium/WebKit after this correction; they failed before it. The separate WebKit invitation-focus case passed locally; its CI-only failure remains unclassified. Main’s corresponding WebKit shard passed.
Generated with Codex