add deployment-wide user directory for user search - #474
Conversation
github-actions
Bot
added
workshop/frontend
Preview:
|
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
| const directoryOpen = selectedUser === null && directoryQuery !== '' | ||
| const canInviteUser = selectedUser !== null || ( | ||
| directoryQuery !== '' && | ||
| directory.status === 'ready' && |
There was a problem hiding this comment.
[P2] Keep exact-ID invites available when lookup fails. This condition permits a raw query only after a successful empty result. When searchUsers() rejects, the status becomes failed, so both Enter and Invite become no-ops even though overseer.addCollaborator() can still resolve a known username/email without the directory. A transient failure of the new singleton therefore regresses the existing direct-invite flow; allow raw submission in the failed state as the fallback.
| ...(currentUser ? [currentUser.id] : []), | ||
| ...collaborators.map(({ profile }) => profile.id), | ||
| ], [collaborators, currentUser]) | ||
| const directoryOpen = selectedUser === null && directoryQuery !== '' |
There was a problem hiding this comment.
[P2] Close the result popover when the combobox loses focus. directoryOpen depends only on a nonempty query and no selection, so after typing, the list remains rendered when the user tabs or clicks into the role picker, share-link controls, or the rest of the modal. It can keep overlaying those following controls indefinitely, especially at the full mobile width. Track combobox focus/outside interaction (and Escape) so leaving the search dismisses the popover while retaining the query.
|
Posted 2 actionable inline findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 3 potential issues.
3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| ON CONFLICT (id) DO UPDATE SET name = excluded.name, search_text = excluded.search_text, | ||
| rev = excluded.rev | ||
| WHERE excluded.rev > users.rev`, | ||
| record.id, record.name, `${record.id}\n${record.name}`.toLowerCase(), rev); |
There was a problem hiding this comment.
🟡 Search crosses profile field boundaries
searchUsers can match a newline-containing query across the stored id and name. It returns users whose individual fields contain no such substring.
Learn more
The directory contract searches for a substring of either the user id or display name. Joining both fields with a separator creates additional substrings spanning the join. The RPC accepts arbitrary strings, so the separator itself does not prevent such queries.
Example: For id ada@example.com and name Grace, query com\ngra matches search_text. Neither ada@example.com nor Grace contains that query, so the user must not be returned.
Recommended fix: Store normalized id and name in separate columns and apply instr to each column independently. Derive ranking from the valid per-field positions rather than a concatenated offset.
Was this helpful? React with 👍 or 👎 to provide feedback.
| searchUsers(query: string, excludeIds: string[]): Promise<UserDirectoryRecord[]> { | ||
| return retryOnDoReset(() => this.ctx.exports.UserDirectoryDurableObject.getByName("") | ||
| .searchUsers(query, [this.#userId.name!, ...excludeIds])); |
There was a problem hiding this comment.
🟨 Directory search lacks input bounds
Any authenticated user can send oversized query and excludeIds values to the singleton directory. Repeated requests can exhaust its CPU or memory and disrupt deployment-wide search.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Adds a central user directory to support platform-wide user search. This is implemented as a singleton DO in the workshop backend that stores a copy of user metadata from each user DO. When user metadata is updated in the user DO, the user DO propagates those changes through to the directory DO by calling the
syncUserRPC method. The directory is incrementally backfilled with existing user records on next login.The backend implementation is as simple as possible. DO SQLite supports FTS5 for full-text search, but this didn't seem worth the complexity. Local benchmarks showed that it was sometimes slower than a full table scan with substring search, and simple substring search is fine for our use case.