human chat @mentions - #2159
Open
webguru-hypha wants to merge 36 commits into
Open
human chat @mentions#2159webguru-hypha wants to merge 36 commits into
webguru-hypha wants to merge 36 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds end-to-end Matrix intentional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Composer as Composer (client)
participant Picker as Mention Picker UI
participant Hook as useMatrixUserIdsByPrivySubs
participant Provider as MatrixProvider
participant Server as App Server
participant DB as Database
User->>Composer: Type "@" + query
Composer->>Picker: open picker / request candidates
Picker->>Hook: request mapping for privySubs (if needed)
Hook->>Server: getMatrixUserIdsByPrivySubsAction(privyUserIds)
Server->>DB: findMatrixUserIdsByPrivyUserIds(...)
DB-->>Server: return mappings
Server-->>Hook: mappings
Hook-->>Picker: mapping results
User->>Picker: select candidate (matrixUserId)
Picker-->>Composer: selected matrixUserId
Composer->>Provider: sendMessage(content, mentionUserIds)
Provider->>Provider: mergeMatrixMentionsIntoContent(...)
Provider->>Matrix Server: send m.room.message (includes m.mentions.user_ids)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
alexprate
marked this pull request as ready for review
April 18, 2026 10:23
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/requirements/Features/human-chat-mentions/requirements.md`:
- Around line 116-119: The policy matrix currently conflates the "Use category
default" and "Only `@mentions`" options, causing ambiguity; update the mapping
table so "Use category default" and "Only `@mentions`" are represented as two
distinct choices (keep the radio option names exactly: "Use category default"
and "Only `@mentions`"), ensure each maps to its own behavior and persistence key
(so the table shows separate mappings for notifications/unread counts and
mention highlighting), and mirror this change in the other instance of the table
(the block that currently mirrors lines ~210-214) so validation and persistence
checks can unambiguously reference each option.
- Around line 94-97: The requirements leave thread-mode vs room-level unread
counters ambiguous for HumanChatPanelTabs and HumanChatPanelHeader (FR-BADGE-1 /
FR-BADGE-2); update the spec to explicitly define badge semantics: state whether
badge counts come from room highlight counts, thread-specific highlight counts,
or a merged value (e.g., room highlights + thread highlights or thread-only when
in thread mode), and specify capping behavior (e.g., "99+"), scope (room vs
thread vs global), and how the bell icon mirrors the same computed value;
reference HumanChatPanelTabs and HumanChatPanelHeader and add a short example or
decision rule clarifying how thread-mode toggling updates the badge to avoid
overcounting.
- Around line 243-249: Section 10 currently lists four unresolved product
decisions (Self-mention, Category default, Bell panel vs aside route, Thread
notification) that block implementation; update the spec by making explicit
decisions for each item (or define a short-term default and an owner/ticket for
longer discussion): decide whether self-mentions are allowed in the picker or
excluded, specify the source/location of the “category” until first-class
categories exist, choose embedded NotificationCentreForm vs navigation to
`@aside/notification-centre` for the bell panel, and state whether v1 supports
room-level only or requires client-side filtering for thread notifications;
annotate the section with the chosen default, rationale in one sentence, and
link or assign an owner/ticket for any items deferred.
- Around line 159-160: The ordered list items referencing reply behavior
("m.in_reply_to") and media-only caption merging ("m.mentions") trigger
markdownlint MD029; update the numbering style to match the project's configured
ordered-list rule (either make every item start with "1." or renumber
sequentially) so the list markers are consistent (e.g., replace "3." and "4."
with the appropriate style), keeping the existing text about preserving reply
relation and merging m.mentions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 35a4ee48-70ab-4c9b-ad2c-6d7ccba083ac
📒 Files selected for processing (1)
docs/requirements/Features/human-chat-mentions/requirements.md
alexprate
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/epics/src/coherence/components/coherence-block.tsx (1)
52-78:⚠️ Potential issue | 🔴 CriticalVerify signal navigation fallback when the Human Chat kill-switch is active.
When
HYPHA_DISABLE_HUMAN_CHAT=true,layout.tsxpassesright={undefined}toPanelWrapLayout, so the right sidebar is never rendered. However,coherence-block.tsxunconditionally callsuseHumanChatPanel()and always passesonSignalClick={handleSignalClick}toSignalGrid, preventing theLinkfallback (basePath/${signal.slug}) from being used. When a signal is clicked,openCoherenceChat()executes and sets state, but since the right panel slot was never passed to the layout, the panel cannot open—resulting in a silent failure with no navigation.Either:
- Conditionally pass
onSignalClickbased onhumanChatEnabled, preserving theLinkfallback when disabled, OR- Provide an alternative navigation mechanism in
openCoherenceChat()for the disabled case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/coherence/components/coherence-block.tsx` around lines 52 - 78, The Signal click currently always invokes useHumanChatPanel().openCoherenceChat (via handleSignalClick) which fails silently when the right-panel kill-switch is active; change coherence-block.tsx so the onSignalClick prop is only passed when human chat is enabled (or have handleSignalClick gate its behavior by checking humanChatEnabled): obtain the humanChatEnabled flag (or read the same env/config used by layout), and either (A) only pass onSignalClick={handleSignalClick} to SignalSection/SignalGrid when humanChatEnabled is true so the Link fallback (basePath/${signal.slug}) is used otherwise, or (B) update handleSignalClick to check humanChatEnabled and, when false, navigate to `${basePath}/${signal.slug}` (e.g., router.push or window.location) instead of calling openCoherenceChat; reference useHumanChatPanel, openCoherenceChat, handleSignalClick, and SignalSection/SignalGrid when making the change.packages/feature-flags/src/index.ts (1)
25-55:⚠️ Potential issue | 🟡 MinorToolbar override for
enable-human-chatis read but the flag is no longer in discovery.
getEnableHumanChat(lines 113–132) still honors a Vercel toolbar override for the keyenable-human-chat, butenableHumanChatwas removed fromflagDefinitionsForDiscovery. Without a discovery entry, the Vercel Flags toolbar won't advertise this flag, so the toolbar-driven kill path documented in the new JSDoc becomes essentially unreachable through the toolbar UI (only via manually crafted override cookies). Either re-add a discovery entry (withdefaultValue: trueand a kill-switch description), or drop the override read to avoid implying a capability the toolbar no longer exposes.As per coding guidelines for
packages/feature-flags/**: "Flags have clear naming conventions and documentation" and "Cleanup path exists for removing old flags."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/feature-flags/src/index.ts` around lines 25 - 55, The toolbar override for key "enable-human-chat" referenced in getEnableHumanChat still exists but the flag was removed from flagDefinitionsForDiscovery; restore consistency by re-adding a discovery entry named enableHumanChat in flagDefinitionsForDiscovery with key: 'enable-human-chat', defaultValue: true, a clear kill-switch description (e.g., "Kill switch to disable human chat UI via Vercel Flags toolbar"), origin: 'hypha' as const, and options: undefined as undefined; alternatively, if you prefer removing toolbar exposure, update getEnableHumanChat to stop reading the toolbar override for 'enable-human-chat' so no UI-implied flag remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/requirements/Features/human-chat-mentions/requirements.md`:
- Around line 72-76: FR-COMP-2 currently restricts space-room chat suggestions
to Matrix joined members (room.getJoinedMembers()) which conflicts with the PR
adding Hypha roster candidates via Person.sub → matrix_user_links; update
FR-COMP-2 (and clarify FR-COMP-3 if needed) to allow inclusion of Hypha
"Members" tab roster candidates (e.g., Person.sub mapped through
matrix_user_links) in the suggestion list for space chats while retaining the v1
rule to exclude the current user; explicitly state that
bridged-only/profile-backed users linked via Person.sub → matrix_user_links are
valid candidates and that room.getJoinedMembers() remains the default source if
no roster is available.
In `@packages/core/src/matrix/mentions.ts`:
- Around line 15-16: The MATRIX_MXID_IN_PLAIN_TEXT regex currently allows
homeserver colons but greedily captures a trailing punctuation colon (e.g.
"@alice:matrix.org: hello"); update the constant MATRIX_MXID_IN_PLAIN_TEXT to
keep permitting colons inside the homeserver portion (so bridged/privy MXIDs
still match) but add a negative lookahead after the homeserver capture that
prevents including a colon that is immediately followed by whitespace or
end-of-string (i.e., disallow a trailing punctuation colon), ensuring internal
colons remain valid while punctuation colons are excluded.
In `@packages/epics/src/common/human-chat-panel/human-chat-mention-token.ts`:
- Around line 37-38: The regex that validates the mention query uses /\w/ which
is ASCII-only and rejects Unicode names; update the validation in
human-chat-mention-token by replacing the test /^[\w.=\-/:@]*$/i with a
Unicode-aware pattern using Unicode property escapes (e.g. \p{L} and \p{N}) and
the u flag so characters like é, ö and apostrophes are allowed, keeping the
MAX_QUERY_LEN and afterAt slicing logic unchanged; then add unit tests for
queries such as "José", "Zoë", "O'Connor" and names with diacritics to ensure
the mention picker no longer closes for these inputs.
In `@packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx`:
- Around line 752-771: openMentionPicker is inserting a second '@' when the
caret is already immediately after one; update the logic inside
openMentionPicker (referencing textareaRef, value, needsSpace, prefix, caret,
onChange, syncAtState) so that if before.endsWith('@') you do not append another
'@' (only add a leading space when needed), compute next and caret accordingly,
then call onChange(next) and requestAnimationFrame to focus,
setSelectionRange(caret, caret), autoResize(), and syncAtState(next, caret) with
the corrected caret position.
- Around line 1193-1280: The capture handlers are intercepting Enter even when a
focused control (attach/mic/emoji buttons, menu items, spoiler/delete, etc.)
should receive it; update handleAttachMenuContentEnter,
handleDraftAttachmentsKeyDownCapture, and handleComposerShellKeyDownCapture to
bail out if the event target is a focusable control by adding a guard that
checks e.target is an HTMLElement and returns when it's an HTMLButtonElement,
HTMLInputElement, HTMLTextAreaElement, or has contentEditable="true" or a
semantic role like "button"/"menuitem"/"option"/"link" (or otherwise focusable),
placing this check before e.preventDefault()/sendMessage so Enter activates the
focused control instead of sending the message.
In `@packages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsx`:
- Around line 308-321: The effect in useLayoutEffect currently calls
onConsumedScrollTarget unconditionally which clears scrollTargetEventId even
when the target DOM row wasn't found; change it so that after resolving esc and
querying containerRef.current you only call onConsumedScrollTarget() and set
stickToBottomRef.current = false after a successful scroll (i.e., when row
instanceof HTMLElement and scrollIntoView was invoked), leaving the id intact if
no row was found so subsequent renders/timelineRows updates can retry scrolling;
locate this logic around the useLayoutEffect that references
scrollTargetEventId, containerRef, timelineRows, onConsumedScrollTarget, and
stickToBottomRef and move the consumption and stick-to-bottom update into the
successful-path only.
In `@packages/epics/src/common/human-chat-panel/human-chat-panel-tabs.tsx`:
- Around line 80-117: Tab button labels hide unread counts because the badge
spans are aria-hidden; update the tab rendering in the tabs.map loop (the button
with id `chat-tab-${tab.key}` and props `aria-selected`/`aria-controls`) to
include an accessible label: when `chatBadgeLabel` or `mentionBadgeLabel` is
present, set a descriptive `aria-label` on the button (e.g., `${tab.label},
${chatBadgeLabel} unread` or translated equivalent) so screen readers announce
the count; keep the visual badge aria-hidden but ensure the `aria-label` uses
the app's i18n translation utility and includes `tab.label` plus the badge count
for both the 'chat' and 'mentions' cases.
In `@packages/epics/src/common/human-chat-panel/matrix-room-member-display.ts`:
- Around line 8-13: The current checks on the trimmed display name (variable l
compared against matrixUserId) only detect synthetic prefixes that include
"_privy_" (e.g. /^prev_privy_/), so change the prefix checks to match the
configured synthetic prefixes themselves (e.g. /^prev_/i and /^prod_/i) and also
include a generic privy check like /^privy_/i or the existing /privy_did_privy/i
if still needed; update the three if statements that test l (the ones with
/^prev_privy_/i, /^prod_privy_/i, /privy_did_privy/i) to instead test for the
correct prefix patterns so display names like "prev_123" or "prod_bridge_user"
are treated as synthetic/trusted when compared to matrixUserId.
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 539-589: The message renderer uses resolveMemberLabel which only
checks Matrix room membership and fallbacks, causing roster-only labels built in
mentionCandidates to be lost; fix by surfacing and prioritizing roster labels:
when constructing mentionCandidates (in mentionCandidates useMemo) store the
roster label via personRosterLabel (e.g., add privySub and/or a rosterLabel
field alongside avatarUrl), and update resolveMemberLabel to first check that
roster-derived label map (or accept the privySub/rosterLabel on the mention
payload) before falling back to matrixMemberDisplayLabel/mxid; ensure you
reference mentionCandidates, personRosterLabel, subToMatrixUserId, and
resolveMemberLabel when making these changes so selected mention pills render
the same Hypha-profile labels.
- Around line 600-609: The current mentionPickerEnabled useMemo only checks
joined Matrix members (via client.getRoom(...).getJoinedMembers()) so it returns
false when there are roster-backed candidates; change the predicate to check for
any mention candidates (joined members OR the roster-backed candidates computed
earlier for the Members tab) and return true if either list has length>0, and
include the roster/candidates variable in the useMemo dependency array alongside
client, roomId, currentUserId, and mentionMembershipEpoch so the picker enables
whenever real mention candidates exist.
In `@packages/feature-flags/src/index.ts`:
- Around line 106-132: getEnableHumanChat currently defaults to true which
violates the feature-flags guideline that features must default to off; change
the function so that when no toolbar override, cookie, legacy flag, or env var
is present it returns false instead of true, and also simplify the toolbar
override handling by replacing the two-branch check (if toolbarHumanChat ===
true / === false) with a single conditional like "if (toolbarHumanChat !==
undefined) return toolbarHumanChat;"; update the JSDoc/comment to reflect the
opt-out kill-switch semantics and ensure references to HYPHA_DISABLE_HUMAN_CHAT
remain accurate.
In `@packages/i18n/src/messages/de.json`:
- Line 1670: The ARIA label for unread mentions under the key
"mentionInboxBellAria" is not pluralized; replace the plain string with ICU
plural syntax using the count variable so singular reads "1 ungelesene
Erwähnung" and other counts use "ungelesene Erwähnungen" (e.g. use "{count,
plural, one {# ungelesene Erwähnung} other {# ungelesene Erwähnungen}}"). Ensure
the JSON value for mentionInboxBellAria uses that ICU plural pattern and keeps
the same "count" interpolation token.
In `@packages/i18n/src/messages/en.json`:
- Line 1671: The aria label value for the key mentionInboxBellAria currently
uses a plain string ("{count} unread mentions") which is incorrect for singular;
update the value to an ICU pluralization message that uses the count variable
and provides distinct forms for one (singular) and other (plural) so screen
readers announce "1 unread mention" vs "2 unread mentions" (use the
mentionInboxBellAria key and the {count, plural, ...} pattern to implement
this).
In `@packages/i18n/src/messages/es.json`:
- Line 1670: The aria label value for the key mentionInboxBellAria must use ICU
pluralization instead of a fixed string; update the value of
mentionInboxBellAria to an ICU plural format that handles singular and plural
(e.g. include at least the "one" and "other" forms, and optionally "=0") using
the {count, plural, one {...} other {...}} pattern so the label correctly reads
"1 mención sin leer" for count=1 and the plural for other counts.
In `@packages/i18n/src/messages/fr.json`:
- Line 1670: The ARIA label "mentionInboxBellAria" is not pluralized and
displays "1 mentions non lues"; update the value to use ICU plural syntax
expected by next-intl so it chooses the correct singular/plural form, e.g. use
the {count, plural, one {# mention non lue} other {# mentions non lues}} pattern
for the "mentionInboxBellAria" message key so the label is grammatically correct
for count=1 and other counts.
In `@packages/i18n/src/messages/pt.json`:
- Line 1670: The aria label for unread mentions (key mentionInboxBellAria) must
use ICU pluralization so it shows "menção" for a single unread and "menções" for
others; update the value to an ICU plural message that includes the {count}
placeholder and defines at least the "one" and "other" forms (e.g., one =>
singular "menção não lida", other => plural "menções não lidas") to ensure
correct grammar for count = 1 and other counts.
---
Outside diff comments:
In `@packages/epics/src/coherence/components/coherence-block.tsx`:
- Around line 52-78: The Signal click currently always invokes
useHumanChatPanel().openCoherenceChat (via handleSignalClick) which fails
silently when the right-panel kill-switch is active; change coherence-block.tsx
so the onSignalClick prop is only passed when human chat is enabled (or have
handleSignalClick gate its behavior by checking humanChatEnabled): obtain the
humanChatEnabled flag (or read the same env/config used by layout), and either
(A) only pass onSignalClick={handleSignalClick} to SignalSection/SignalGrid when
humanChatEnabled is true so the Link fallback (basePath/${signal.slug}) is used
otherwise, or (B) update handleSignalClick to check humanChatEnabled and, when
false, navigate to `${basePath}/${signal.slug}` (e.g., router.push or
window.location) instead of calling openCoherenceChat; reference
useHumanChatPanel, openCoherenceChat, handleSignalClick, and
SignalSection/SignalGrid when making the change.
In `@packages/feature-flags/src/index.ts`:
- Around line 25-55: The toolbar override for key "enable-human-chat" referenced
in getEnableHumanChat still exists but the flag was removed from
flagDefinitionsForDiscovery; restore consistency by re-adding a discovery entry
named enableHumanChat in flagDefinitionsForDiscovery with key:
'enable-human-chat', defaultValue: true, a clear kill-switch description (e.g.,
"Kill switch to disable human chat UI via Vercel Flags toolbar"), origin:
'hypha' as const, and options: undefined as undefined; alternatively, if you
prefer removing toolbar exposure, update getEnableHumanChat to stop reading the
toolbar override for 'enable-human-chat' so no UI-implied flag remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0e5d5937-d24b-49aa-b721-79426942c831
📒 Files selected for processing (44)
apps/web-e2e/src/human-chat-panel-avatar.spec.tsapps/web-e2e/src/human-chat-panel-feature-flag.spec.tsapps/web-e2e/src/human-chat-panel-header.spec.tsapps/web-e2e/src/human-chat-panel-members.spec.tsapps/web-e2e/src/human-chat-panel-resize.spec.tsapps/web-e2e/src/human-chat-panel-space-switch.spec.tsapps/web-e2e/src/menu-top-consistent-height.spec.tsapps/web-e2e/src/pages/human-chat-panel.page.tsapps/web-e2e/src/panel-layout.spec.tsapps/web-e2e/src/panels-space-context.spec.tsapps/web/src/app/[lang]/dho/[id]/@tab/coherence/page.tsxdocs/requirements/Features/human-chat-mentions/requirements.mdpackages/cookie/src/constants.tspackages/core/src/matrix/__tests__/mentions.test.tspackages/core/src/matrix/client/hooks/index.tspackages/core/src/matrix/client/hooks/use-matrix-user-ids-by-privy-subs.tspackages/core/src/matrix/client/providers/matrix-provider.tsxpackages/core/src/matrix/edit-room-message-media-caption.tspackages/core/src/matrix/index.tspackages/core/src/matrix/mentions.tspackages/core/src/matrix/rich-reply.tspackages/core/src/matrix/server/actions.tspackages/core/src/matrix/server/queries.tspackages/core/src/matrix/types.tspackages/epics/src/coherence/components/coherence-block.tsxpackages/epics/src/common/human-chat-panel/human-chat-mention-candidate-row.tsxpackages/epics/src/common/human-chat-panel/human-chat-mention-token.tspackages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-header.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-mention-inbox.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-tabs.tsxpackages/epics/src/common/human-chat-panel/index.tspackages/epics/src/common/human-chat-panel/matrix-chat-unread.tspackages/epics/src/common/human-chat-panel/matrix-room-member-display.tspackages/epics/src/common/human-chat-panel/parse-simple-matrix-html.tsxpackages/epics/src/common/human-right-panel.tsxpackages/feature-flags/src/index.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
💤 Files with no reviewable changes (1)
- apps/web-e2e/src/human-chat-panel-space-switch.spec.ts
alexprate
self-requested a review
April 19, 2026 00:26
alexprate
approved these changes
Apr 19, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/feature-flags/src/index.ts (1)
25-55:⚠️ Potential issue | 🟡 MinorKeep the Human Chat flag discoverable if toolbar rollback is documented.
Line 115 documents Vercel Flags Toolbar rollback for
enable-human-chat, and lines 118-120 still read that override, but the flag is absent fromflagDefinitionsForDiscovery. Operators may not see the toggle in the toolbar during rollback. Re-add a discovery entry or remove the toolbar rollback claim from the JSDoc. As per coding guidelines forpackages/feature-flags/**: "Flags have clear naming conventions and documentation."Also applies to: 115-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/feature-flags/src/index.ts` around lines 25 - 55, flagDefinitionsForDiscovery is missing the discovery entry for the human chat flag referenced in the toolbar rollback docs; either re-add the flag object or remove the toolbar rollback mention. To fix, add a new entry under flagDefinitionsForDiscovery named (e.g.) enableHumanChat with key: 'enable-human-chat', defaultValue: false, a brief description matching the toolbar use, origin: 'hypha' as const, and options: undefined as undefined so operators can discover the toggle in the toolbar; alternatively, if the flag is intentionally removed, delete the toolbar rollback lines that reference `enable-human-chat` so docs are consistent.packages/i18n/src/messages/de.json (1)
1628-1672:⚠️ Potential issue | 🟡 MinorSeparate numeric counts from capped display labels for ICU plural messages.
mentionInboxBellAriais already broken: it receives'99+'(a string) when count is capped, but next-intl's ICU plural requires a numeric argument for correct plural selection. ConvertingtabChatWithMentionCountandtabMentionsWithMentionCountto ICU plural without fixing this architectural issue will introduce the same defect.Pass the numeric count for plural logic and a separate display label (
countLabelor similar) for the UI string. Alternatively, use dedicated capped translation keys (e.g.,"tabChatWithMentionCountCapped") that don't rely on ICU plural.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/src/messages/de.json` around lines 1628 - 1672, The ICU plural messages (mentionInboxBellAria, tabChatWithMentionCount, tabMentionsWithMentionCount) must use a numeric count for plural selection and a separate display label for the capped UI value: change callers to pass count (number) for plural logic and pass countLabel (string, e.g., "99+") for the rendered text, then update the translations to use {count, plural, ...} for plural-sensitive parts and {countLabel} where the capped string should appear; alternatively add dedicated capped keys (e.g., tabChatWithMentionCountCapped) and use those where the UI receives a capped string so no ICU plural receives a non-numeric value.packages/i18n/src/messages/pt.json (1)
1628-1672:⚠️ Potential issue | 🟡 MinorFix ICU plural counting for capped badge labels.
The
mentionInboxBellAriamessage uses ICU plural syntax but receives a string'99+'when the mention count is capped (line 149, human-chat-panel-mention-inbox.tsx:count: countIsCapped ? '99+' : unreadCount). ICU plural requires a numeric count to select the correct grammatical form; passing a string breaks this logic.Additionally,
tabChatWithMentionCountandtabMentionsWithMentionCountreceive capped string labels ('99+') from the badge label logic, which causes grammatical issues in Portuguese (e.g., "99+ menções não lidas" is incorrect; it should vary based on singular/plural).To resolve: Pass a numeric
countvalue for plural selection alongside a separatecountLabeldisplay string, or create dedicated message keys for capped badge labels (e.g.,mentionInboxBellAriaCapped,tabChatWithMentionCountCapped).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/src/messages/pt.json` around lines 1628 - 1672, The ICU pluralization is broken because badge logic passes a capped string ('99+') into plural keys; update the rendering code (where countIsCapped is used in human-chat-panel-mention-inbox.tsx) to always pass a numeric "count" to pluralized message keys (mentionInboxBellAria, tabChatWithMentionCount, tabMentionsWithMentionCount) and supply a separate "countLabel" or "countDisplay" string for visual badges when capped, or alternatively add new capped message keys (mentionInboxBellAriaCapped, tabChatWithMentionCountCapped, tabMentionsWithMentionCountCapped) and use those when countIsCapped is true so ICU plural selectors receive a number while the UI shows '99+'.packages/i18n/src/messages/en.json (1)
1629-1674:⚠️ Potential issue | 🟠 MajorPass numeric counts to plural formatters; use string labels for display-only text.
The
mentionInboxBellAriamessage uses ICU plural formatting ({count, plural, ...}), but the caller inHumanChatPanelMentionBell(line 149) passescountIsCapped ? '99+' : unreadCount—when capped, this sends a non-numeric string to the formatter, breaking plural selection.Additionally, the tab aria messages
tabChatWithMentionCountandtabMentionsWithMentionCountalways say "mentions" (plural form) regardless of count, so when the badge shows "1", the aria-label reads "1 unread mentions".Create a separate key for the capped/string-label case in the bell component, and either make tab text grammar-neutral or use numeric counts for those as well. Ensure plural messages receive only numeric values.
🌐 Suggested locale adjustment
- "tabChatWithMentionCount": "{tabLabel}, {count} unread mentions", + "tabChatWithMentionCount": "{tabLabel}, unread mentions: {count}", ... - "tabMentionsWithMentionCount": "{tabLabel}, {count} unread mentions", + "tabMentionsWithMentionCount": "{tabLabel}, unread mentions: {count}", ... "mentionInboxBellAria": "{count, plural, one {# unread mention} other {# unread mentions}}", + "mentionInboxBellAriaCapped": "{countLabel} unread mentions",Then update
HumanChatPanelMentionBell(line 146–151) to use the capped key when appropriate:aria-label={ unreadCount > 0 ? countIsCapped ? t('mentionInboxBellAriaCapped', { countLabel: '99+' }) : t('mentionInboxBellAria', { count: unreadCount }) : t('mentionInboxBellAriaEmpty') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/src/messages/en.json` around lines 1629 - 1674, The locale keys send non-numeric strings into an ICU plural formatter and use incorrect plural wording for singular counts; update locales and component usage so plural formatters only receive numbers and capped labels use a separate string. Add a new key (e.g., "mentionInboxBellAriaCapped") that accepts a string label (countLabel) for the capped case, keep "mentionInboxBellAria" numeric (count) for ICU plural, and change HumanChatPanelMentionBell to call the capped key when countIsCapped and the numeric key otherwise; also adjust "tabChatWithMentionCount" and "tabMentionsWithMentionCount" to be grammar-neutral (e.g., "{tabLabel}, {count} unread mention(s)" or use ICU plural with numeric {count}) so they do not always force the plural form when count === 1.packages/i18n/src/messages/fr.json (1)
1628-1673:⚠️ Potential issue | 🟠 MajorFix ICU plural formatter mismatch: pass numeric values, not capped string labels.
mentionInboxBellAriais an ICU plural message in all locales but receives'99+'(string) instead of a numericcountwhencountIsCappedis true inHumanChatPanelMentionBell. This breaks next-intl's plural form selection.The tab aria messages (
tabChatWithMentionCount,tabMentionsWithMentionCount) also receive string badge labels and produce grammatically incorrect output for singular counts (e.g., "1 mentions non lues" in French).Solutions:
- Add separate message keys for capped display (e.g.,
mentionInboxBellAriaCapped) that accept a string label instead of a numeric count, and update both callers to use numeric paths when available.- Or redesign tab messages to use plural syntax instead of simple interpolation.
Example fix for mention bell (requires caller update)
Add to all locale files (fr.json, en.json, de.json, es.json, pt.json):
"mentionInboxBellAria": "{count, plural, one {# mention non lue} other {# mentions non lues}}", +"mentionInboxBellAriaCapped": "{countLabel} mentions non lues",Update caller in
human-chat-panel-mention-inbox.tsx:aria-label={ unreadCount > 0 ? countIsCapped ? t('mentionInboxBellAriaCapped', { countLabel: '99+' }) : t('mentionInboxBellAria', { count: unreadCount }) : t('mentionInboxBellAriaEmpty') }Same pattern applies to
tabChatWithMentionCountandtabMentionsWithMentionCountinhuman-chat-panel-tabs.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/src/messages/fr.json` around lines 1628 - 1673, The ICU plural keys (mentionInboxBellAria, tabChatWithMentionCount, tabMentionsWithMentionCount) are being fed string badge labels like "99+" which breaks plural selection; add new capped-string variants (e.g., mentionInboxBellAriaCapped, tabChatWithMentionCountCapped, tabMentionsWithMentionCountCapped) that accept a string param (countLabel) in all locale JSONs and update the callers in HumanChatPanelMentionBell and HumanChatPanelTabs to use the capped key when countIsCapped is true and the numeric plural key (mentionInboxBellAria / tab* with {count}) otherwise, ensuring plural keys always receive a numeric count.packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx (1)
1067-1096:⚠️ Potential issue | 🟠 MajorUse
stop()for user-initiated stops andabort()for send/discard interrupts.When pressing the stop button, the current code calls
abort(), which per MDN does not attempt to return the final recognized result. This can trigger theonerrorhandler without a final transcript, causingdictationErrorto display and losing the captured audio. For normal user stops, usestop()instead, which finalizes and delivers any captured audio. Reserveabort()for send/discard contexts where interruption should discard pending results.The app already distinguishes these contexts via
dictationInterruptForSendRefinprepareSendSession(). Apply this pattern tostopDictation()to call the appropriate method:
- User stop (button click at lines 1113, 2028):
stop()- Send interrupt (line 1089 via
prepareSendSession):abort()🎙️ Proposed fix
- const stopDictation = useCallback(() => { + const stopDictation = useCallback((discard = false) => { const r = speechRecognitionRef.current; if (r) { try { - r.abort(); + if (discard) { + r.abort(); + } else { + r.stop(); + } } catch { try { - r.stop(); + r.abort(); } catch { // ignore } } - speechRecognitionRef.current = null; + if (discard) { + speechRecognitionRef.current = null; + } } - dictationPrefixRef.current = ''; - setIsDictating(false); + if (discard) { + dictationPrefixRef.current = ''; + setIsDictating(false); + } }, []); @@ dictationInterruptForSendRef.current = true; discardVoiceDraftIfIdleRef.current = true; voiceAsAttachmentRef.current = false; - stopDictation(); + stopDictation(true); stopVoiceRecording(); }, [stopDictation, stopVoiceRecording]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx` around lines 1067 - 1096, The stop logic should call stop() for user-initiated stops and abort() for send/discard interrupts: modify stopDictation() to check dictationInterruptForSendRef.current and call r.abort() when true and r.stop() when false (then clear speechRecognitionRef and reset dictationPrefixRef/isDictating as before), leaving prepareSendSession() to set dictationInterruptForSendRef.current = true (it already does) before calling stopDictation(); also ensure any user stop handlers set dictationInterruptForSendRef.current = false before invoking stopDictation() so user button clicks use stop() not abort().
♻️ Duplicate comments (2)
packages/feature-flags/src/index.ts (1)
106-135:⚠️ Potential issue | 🟠 MajorDefault-on still violates the feature-flag safety guideline.
getEnableHumanChat()now defaults totrue, which exposes the Matrix provider/panel and token endpoint unless a kill switch is configured. If this opt-out rollout is approved, update the repository feature-flag guideline or add an explicit exception/runbook; otherwise keep the safe defaultfalse. As per coding guidelines forpackages/feature-flags/**: "Default values are safe (features default to off)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/feature-flags/src/index.ts` around lines 106 - 135, getEnableHumanChat currently returns true by default which violates the safe "off-by-default" feature-flag guideline; change the behavior so that getEnableHumanChat (and its use of readBooleanOverride/getVercelToolbarFlagOverrides and cookie/env checks like HYPHA_DISABLE_HUMAN_CHAT, HYPHA_ENABLE_HUMAN_CHAT, NEXT_PUBLIC_DISABLE_HUMAN_CHAT, NEXT_PUBLIC_ENABLE_HUMAN_CHAT) only enables human chat when an explicit positive override is present and otherwise returns false, or if you must keep the opt-out rollout add an explicit documented exception/runbook and update the repository feature-flag guideline to reference that runbook instead of changing the function behavior.packages/epics/src/common/human-chat-panel/matrix-room-member-display.ts (1)
16-21:⚠️ Potential issue | 🟠 MajorMatch generic
prev_synthetic labels too.The PR objective calls out
prev_as a known synthetic prefix, but this only matchesprev_privy. Preview bridged names likeprev_bridge_usercan still leak into mention rows and fallback labels.🐛 Proposed fix
function stemLooksLikeBridgedPrivyLocalpart(stem: string): boolean { const s = stem.trim(); - if (/^prev_privy/i.test(s) || /^prod_privy/i.test(s)) return true; + if (/^prev_/i.test(s) || /^prod_privy/i.test(s)) return true; /** Production Matrix bridge locals often start with `prod_` (not always `prod_privy`). */ if (/^prod_/i.test(s)) return true; return false; } ... const looksSynthetic = - /privy|_did_|^prod_|^prev_privy|^prod_privy/i.test(local) || + /privy|_did_|^prod_|^prev_|^prod_privy/i.test(local) || local.length > 28;Also applies to: 60-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/human-chat-panel/matrix-room-member-display.ts` around lines 16 - 21, The function stemLooksLikeBridgedPrivyLocalpart currently only matches /^prev_privy/i for the prev_* synthetic prefix; change that to match the generic /^prev_/i (case-insensitive) so any preview bridged names like "prev_bridge_user" are detected, and update the analogous checks elsewhere in the file (the similar prev_/prod_ check around the other occurrence) to use the same /^prev_/i pattern while keeping the existing /^prod_/i and /^prod_privy/i logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/matrix/mentions.ts`:
- Around line 27-49: The function normalizePlainTextMxidCaptureFromMatch fails
to strip a single trailing punctuation colon for port-qualified MXIDs (e.g.
"@alice:matrix.org:8448:") because it checks the port regexp before removing the
colon; change the loop in normalizePlainTextMxidCaptureFromMatch to first set m
= without (i.e., strip one trailing ':'), then if the new m (the `without`
value) matches /:\d+$/ break to avoid removing the port digits—this ensures you
remove only the stray punctuation colon but preserve valid host:port MXIDs and
still stop after a single removal for cases like "@user:hs: hello".
In
`@packages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsx`:
- Around line 935-944: The Matrix mention pill double-prefixes labels because
the span always prepends "@" while resolveMx can return a value starting with
"@"; update the rendering in the seg.kind === 'mxid' branch (the parts.push that
creates the span) to avoid adding an extra "@" by normalizing the resolved label
from resolveMx(seg.full) — e.g., strip a leading "@" from the label if present
(or conditionally prefix "@" only when the label doesn't already start with "@")
before using it in the span.
- Around line 891-919: The regex should avoid matching `@` inside emails by
requiring a left boundary; update localHandleRe (and hence reLocal in
mapPlainFragment) to include a capture for the left-context, e.g.
/(^|\s)@([^\s@]{1,100}?)(?=\s|$|[.,!?;:])/g, and then in mapPlainFragment adjust
match handling to treat mh[1] (the prefix, either empty or a whitespace) as
plain text (push it or include it in the preceding slice) and use mh[2] as the
handle to render the pill; ensure you update indices/last computation
accordingly so the prefix isn’t swallowed or turned into a pill.
In `@packages/epics/src/common/human-chat-panel/matrix-chat-unread.ts`:
- Around line 41-75: countUnreadMentionMessagesForUser currently parses mentions
from each original event's content and skips m.replace events, which can miss
mention edits; change it to resolve the latest replacement content for the
target event before parsing. For each event ev, call
getMessageReplaceTargetEventId(ev) to get targetId; if targetId is non-null,
scan the same timeline (room.getLiveTimeline().getEvents()) for a replacement
event whose getMessageReplaceTargetEventId(...) === targetId and is not
redacted, and use that replacement's getContent() as the wire content; otherwise
fall back to ev.getContent(); then pass that resolved content into
parseMentionUserIdsFromWireContent and continue the rest of the checks in
countUnreadMentionMessagesForUser.
In `@packages/i18n/src/messages/es.json`:
- Around line 1628-1631: The tab mention-count labels (tabChatWithMentionCount
and tabMentionsWithMentionCount) currently hardcode "{count} menciones sin leer"
which is incorrect for singular; change both keys to use ICU pluralization for
{count} (e.g. a plural block handling one and other) and keep {tabLabel}
prefixed — update the string values to use "{tabLabel}, {count, plural, one {#
mención sin leer} other {# menciones sin leer}}" (and include any =0 branch if
desired) so the aria/label logic matches the pluralized bell label.
---
Outside diff comments:
In `@packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx`:
- Around line 1067-1096: The stop logic should call stop() for user-initiated
stops and abort() for send/discard interrupts: modify stopDictation() to check
dictationInterruptForSendRef.current and call r.abort() when true and r.stop()
when false (then clear speechRecognitionRef and reset
dictationPrefixRef/isDictating as before), leaving prepareSendSession() to set
dictationInterruptForSendRef.current = true (it already does) before calling
stopDictation(); also ensure any user stop handlers set
dictationInterruptForSendRef.current = false before invoking stopDictation() so
user button clicks use stop() not abort().
In `@packages/feature-flags/src/index.ts`:
- Around line 25-55: flagDefinitionsForDiscovery is missing the discovery entry
for the human chat flag referenced in the toolbar rollback docs; either re-add
the flag object or remove the toolbar rollback mention. To fix, add a new entry
under flagDefinitionsForDiscovery named (e.g.) enableHumanChat with key:
'enable-human-chat', defaultValue: false, a brief description matching the
toolbar use, origin: 'hypha' as const, and options: undefined as undefined so
operators can discover the toggle in the toolbar; alternatively, if the flag is
intentionally removed, delete the toolbar rollback lines that reference
`enable-human-chat` so docs are consistent.
In `@packages/i18n/src/messages/de.json`:
- Around line 1628-1672: The ICU plural messages (mentionInboxBellAria,
tabChatWithMentionCount, tabMentionsWithMentionCount) must use a numeric count
for plural selection and a separate display label for the capped UI value:
change callers to pass count (number) for plural logic and pass countLabel
(string, e.g., "99+") for the rendered text, then update the translations to use
{count, plural, ...} for plural-sensitive parts and {countLabel} where the
capped string should appear; alternatively add dedicated capped keys (e.g.,
tabChatWithMentionCountCapped) and use those where the UI receives a capped
string so no ICU plural receives a non-numeric value.
In `@packages/i18n/src/messages/en.json`:
- Around line 1629-1674: The locale keys send non-numeric strings into an ICU
plural formatter and use incorrect plural wording for singular counts; update
locales and component usage so plural formatters only receive numbers and capped
labels use a separate string. Add a new key (e.g., "mentionInboxBellAriaCapped")
that accepts a string label (countLabel) for the capped case, keep
"mentionInboxBellAria" numeric (count) for ICU plural, and change
HumanChatPanelMentionBell to call the capped key when countIsCapped and the
numeric key otherwise; also adjust "tabChatWithMentionCount" and
"tabMentionsWithMentionCount" to be grammar-neutral (e.g., "{tabLabel}, {count}
unread mention(s)" or use ICU plural with numeric {count}) so they do not always
force the plural form when count === 1.
In `@packages/i18n/src/messages/fr.json`:
- Around line 1628-1673: The ICU plural keys (mentionInboxBellAria,
tabChatWithMentionCount, tabMentionsWithMentionCount) are being fed string badge
labels like "99+" which breaks plural selection; add new capped-string variants
(e.g., mentionInboxBellAriaCapped, tabChatWithMentionCountCapped,
tabMentionsWithMentionCountCapped) that accept a string param (countLabel) in
all locale JSONs and update the callers in HumanChatPanelMentionBell and
HumanChatPanelTabs to use the capped key when countIsCapped is true and the
numeric plural key (mentionInboxBellAria / tab* with {count}) otherwise,
ensuring plural keys always receive a numeric count.
In `@packages/i18n/src/messages/pt.json`:
- Around line 1628-1672: The ICU pluralization is broken because badge logic
passes a capped string ('99+') into plural keys; update the rendering code
(where countIsCapped is used in human-chat-panel-mention-inbox.tsx) to always
pass a numeric "count" to pluralized message keys (mentionInboxBellAria,
tabChatWithMentionCount, tabMentionsWithMentionCount) and supply a separate
"countLabel" or "countDisplay" string for visual badges when capped, or
alternatively add new capped message keys (mentionInboxBellAriaCapped,
tabChatWithMentionCountCapped, tabMentionsWithMentionCountCapped) and use those
when countIsCapped is true so ICU plural selectors receive a number while the UI
shows '99+'.
---
Duplicate comments:
In `@packages/epics/src/common/human-chat-panel/matrix-room-member-display.ts`:
- Around line 16-21: The function stemLooksLikeBridgedPrivyLocalpart currently
only matches /^prev_privy/i for the prev_* synthetic prefix; change that to
match the generic /^prev_/i (case-insensitive) so any preview bridged names like
"prev_bridge_user" are detected, and update the analogous checks elsewhere in
the file (the similar prev_/prod_ check around the other occurrence) to use the
same /^prev_/i pattern while keeping the existing /^prod_/i and /^prod_privy/i
logic intact.
In `@packages/feature-flags/src/index.ts`:
- Around line 106-135: getEnableHumanChat currently returns true by default
which violates the safe "off-by-default" feature-flag guideline; change the
behavior so that getEnableHumanChat (and its use of
readBooleanOverride/getVercelToolbarFlagOverrides and cookie/env checks like
HYPHA_DISABLE_HUMAN_CHAT, HYPHA_ENABLE_HUMAN_CHAT,
NEXT_PUBLIC_DISABLE_HUMAN_CHAT, NEXT_PUBLIC_ENABLE_HUMAN_CHAT) only enables
human chat when an explicit positive override is present and otherwise returns
false, or if you must keep the opt-out rollout add an explicit documented
exception/runbook and update the repository feature-flag guideline to reference
that runbook instead of changing the function behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 310beacd-b53c-4cd0-902e-12dfff3aeb92
📒 Files selected for processing (17)
docs/requirements/Features/human-chat-mentions/requirements.mdpackages/core/src/matrix/__tests__/mentions.test.tspackages/core/src/matrix/mentions.tspackages/epics/src/common/human-chat-panel/human-chat-mention-token.tspackages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-tabs.tsxpackages/epics/src/common/human-chat-panel/matrix-chat-unread.tspackages/epics/src/common/human-chat-panel/matrix-room-member-display.tspackages/epics/src/common/human-right-panel.tsxpackages/feature-flags/src/index.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/matrix/mentions.ts`:
- Around line 52-54: The current isLikelyMatrixUserId allows whitespace after
the colon (e.g. "@alice:matrix.org extra"); tighten the regex to require no
whitespace after the colon by changing the pattern in isLikelyMatrixUserId to
something like /^@[^\s:]+:[^\s]+$/ (or equivalent) so the whole string must be a
valid single Matrix ID, and update parseMentionUserIdsFromWireContent to
validate and filter each wire-provided ID by calling isLikelyMatrixUserId
(discard invalid IDs) before returning or emitting mention IDs; this ensures
consistency between explicit ID handling and wire content parsing.
In `@packages/epics/src/common/human-chat-panel/matrix-chat-unread.ts`:
- Around line 47-109: countUnreadMentionMessagesForUser currently rescans the
room timeline for every event via wireContentForMentionParse, causing O(n²)
behavior; precompute a map from rootEventId -> latest non-redacted replacement
event once at the start of the function (iterate
room.getLiveTimeline().getEvents() once, use getMessageReplaceTargetEventId and
isRedactedRoomMessageEvent to update the map by timestamp) and then replace
calls to wireContentForMentionParse(room, ev) with a direct lookup into that map
(or change wireContentForMentionParse to accept the precomputed map) so
parseMentionUserIdsFromWireContent uses the cached latest content for each root
id while keeping the rest of the logic in countUnreadMentionMessagesForUser
(including readUpToId compare, room.compareEventOrdering, and
room.hasUserReadEvent) unchanged.
In `@packages/feature-flags/src/index.ts`:
- Around line 56-62: The flag object enableSpaceMemory uses a mixed-case key
'enable-Space-Memory'; change its key property to lowercase kebab-case
'enable-space-memory' and update any other occurrences of that key (including
the second enableSpaceMemory entry elsewhere in the file) as well as any
readBooleanOverride calls that reference the old mixed-case string so toolbar
overrides and discovery keys match exactly; ensure the description/origin remain
unchanged and run tests/typechecks after updating references.
- Around line 138-152: Move the emergency-disable checks to run before honoring
any toolbar override: first read cookies and environment disables
(HYPHA_DISABLE_HUMAN_CHAT and NEXT_PUBLIC_DISABLE_HUMAN_CHAT) and immediately
return false if set; then apply legacy enable logic (HYPHA_ENABLE_HUMAN_CHAT)
and only after those checks call getVercelToolbarFlagOverrides() and
readBooleanOverride() to consider the toolbar flag; ensure
readBooleanOverride('enable-human-chat') cannot override the disables so the
disable paths are non-overridable.
In `@packages/i18n/src/messages/en.json`:
- Around line 1629-1632: The ICU plural messages tabChatWithMentionCount,
tabMentionsWithMentionCount, and mentionInboxBellAria must be split to use a
numeric plural operand and a separate display string; update the JSON entries to
accept both count (numeric) for the plural branch and displayCount for the
visible/badge text (e.g. "{tabLabel}, {count, plural, one {# unread mention}
other {# unread mentions}} ({displayCount})" or similar), then update the
callers human-chat-panel-mention-inbox.tsx (where '99+' is passed),
human-chat-panel-tabs.tsx (where chatBadgeLabel and mentionBadgeLabel are
passed) to pass count: unreadCount (number) and displayCount: badgeLabel
(string) instead of passing the display string into the plural operand.
In `@packages/i18n/src/messages/fr.json`:
- Around line 1628-1631: The ICU plural messages (tabChatWithMentionCount,
tabMentionsWithMentionCount, mentionInboxBellAria) currently use `{count,
plural,...}` but call sites pass display strings like "99+", breaking plural
selection; update the JSON messages to accept both a numeric `count` for plural
selection and a `displayCount` for the badge text (e.g. change templates to use
`{count, plural, one {...} other {...}}` for grammar and insert `{displayCount}`
where the visible badge should appear), apply the same change to the English
counterparts, and update the component call sites (human-chat-panel-tabs and
human-chat-panel-mention-inbox) to pass `count: unreadCount` (numeric) and
`displayCount: badgeLabel` (string, e.g. `countIsCapped ? '99+' :
String(unreadCount)`).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bbf79eb6-8793-4987-b834-22b33e74e02d
📒 Files selected for processing (14)
apps/web/src/app/[lang]/dho/[id]/@tab/coherence/page.tsxapps/web/src/app/[lang]/dho/[id]/@tab/layout.tsxpackages/cookie/src/constants.tspackages/core/src/matrix/__tests__/mentions.test.tspackages/core/src/matrix/mentions.tspackages/epics/src/coherence/components/coherence-block.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsxpackages/epics/src/common/human-chat-panel/matrix-chat-unread.tspackages/feature-flags/src/index.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
Comment on lines
+138
to
+152
| const overrides = await getVercelToolbarFlagOverrides(); | ||
| const toolbarHumanChat = readBooleanOverride(overrides, 'enable-human-chat'); | ||
| if (toolbarHumanChat !== undefined) return toolbarHumanChat; | ||
|
|
||
| const store = await cookies(); | ||
|
|
||
| if (store.get(HYPHA_DISABLE_HUMAN_CHAT)?.value === 'true') { | ||
| return false; | ||
| } | ||
|
|
||
| const legacyEnable = store.get(HYPHA_ENABLE_HUMAN_CHAT)?.value; | ||
| if (legacyEnable === 'false') return false; | ||
|
|
||
| if (process.env.NEXT_PUBLIC_DISABLE_HUMAN_CHAT === 'true') return false; | ||
| if (process.env.NEXT_PUBLIC_ENABLE_HUMAN_CHAT === 'false') return false; |
Contributor
There was a problem hiding this comment.
Make emergency disables non-overridable.
Line 140 returns a toolbar true before checking HYPHA_DISABLE_HUMAN_CHAT or NEXT_PUBLIC_DISABLE_HUMAN_CHAT, so a stale toolbar override can bypass the documented rollback path and still enable Matrix token issuance. Check disable cookie/env first, then apply non-emergency overrides.
🛠️ Proposed fix
export async function getEnableHumanChat(): Promise<boolean> {
+ const store = await cookies();
+
+ if (store.get(HYPHA_DISABLE_HUMAN_CHAT)?.value === 'true') {
+ return false;
+ }
+
+ if (process.env.NEXT_PUBLIC_DISABLE_HUMAN_CHAT === 'true') return false;
+
const overrides = await getVercelToolbarFlagOverrides();
const toolbarHumanChat = readBooleanOverride(overrides, 'enable-human-chat');
if (toolbarHumanChat !== undefined) return toolbarHumanChat;
- const store = await cookies();
-
- if (store.get(HYPHA_DISABLE_HUMAN_CHAT)?.value === 'true') {
- return false;
- }
-
const legacyEnable = store.get(HYPHA_ENABLE_HUMAN_CHAT)?.value;
if (legacyEnable === 'false') return false;
- if (process.env.NEXT_PUBLIC_DISABLE_HUMAN_CHAT === 'true') return false;
if (process.env.NEXT_PUBLIC_ENABLE_HUMAN_CHAT === 'false') return false;
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const overrides = await getVercelToolbarFlagOverrides(); | |
| const toolbarHumanChat = readBooleanOverride(overrides, 'enable-human-chat'); | |
| if (toolbarHumanChat !== undefined) return toolbarHumanChat; | |
| const store = await cookies(); | |
| if (store.get(HYPHA_DISABLE_HUMAN_CHAT)?.value === 'true') { | |
| return false; | |
| } | |
| const legacyEnable = store.get(HYPHA_ENABLE_HUMAN_CHAT)?.value; | |
| if (legacyEnable === 'false') return false; | |
| if (process.env.NEXT_PUBLIC_DISABLE_HUMAN_CHAT === 'true') return false; | |
| if (process.env.NEXT_PUBLIC_ENABLE_HUMAN_CHAT === 'false') return false; | |
| const store = await cookies(); | |
| if (store.get(HYPHA_DISABLE_HUMAN_CHAT)?.value === 'true') { | |
| return false; | |
| } | |
| if (process.env.NEXT_PUBLIC_DISABLE_HUMAN_CHAT === 'true') return false; | |
| const overrides = await getVercelToolbarFlagOverrides(); | |
| const toolbarHumanChat = readBooleanOverride(overrides, 'enable-human-chat'); | |
| if (toolbarHumanChat !== undefined) return toolbarHumanChat; | |
| const legacyEnable = store.get(HYPHA_ENABLE_HUMAN_CHAT)?.value; | |
| if (legacyEnable === 'false') return false; | |
| if (process.env.NEXT_PUBLIC_ENABLE_HUMAN_CHAT === 'false') return false; | |
| return true; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/feature-flags/src/index.ts` around lines 138 - 152, Move the
emergency-disable checks to run before honoring any toolbar override: first read
cookies and environment disables (HYPHA_DISABLE_HUMAN_CHAT and
NEXT_PUBLIC_DISABLE_HUMAN_CHAT) and immediately return false if set; then apply
legacy enable logic (HYPHA_ENABLE_HUMAN_CHAT) and only after those checks call
getVercelToolbarFlagOverrides() and readBooleanOverride() to consider the
toolbar flag; ensure readBooleanOverride('enable-human-chat') cannot override
the disables so the disable paths are non-overridable.
Comment on lines
+1629
to
+1632
| "tabChatWithMentionCount": "{tabLabel}, {count, plural, one {# unread mention} other {# unread mentions}}", | ||
| "tabMembers": "Members", | ||
| "tabMentions": "Mentions", | ||
| "tabMentionsWithMentionCount": "{tabLabel}, {count, plural, one {# unread mention} other {# unread mentions}}", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate mention-count i18n calls and inspect whether `count` is passed as a display string.
rg -n -C5 "tabChatWithMentionCount|tabMentionsWithMentionCount|mentionInboxBellAria" packages/epicsRepository: hypha-dao/hypha-web
Length of output: 3493
🏁 Script executed:
sed -n '1629,1632p; 1673,1674p' packages/i18n/src/messages/en.jsonRepository: hypha-dao/hypha-web
Length of output: 497
Separate numeric plural count from the displayed badge count.
The message keys tabChatWithMentionCount, tabMentionsWithMentionCount, and mentionInboxBellAria use ICU plural syntax ({count, plural, ...}), which expects a numeric operand. However, callers pass display strings:
'99+'in human-chat-panel-mention-inbox.tsx:149chatBadgeLabelin human-chat-panel-tabs.tsx:90mentionBadgeLabelin human-chat-panel-tabs.tsx:95
Pass a numeric count for plural evaluation and a separate displayCount for the accessible label text. Update message definitions to reference both:
Proposed message shape
- "tabChatWithMentionCount": "{tabLabel}, {count, plural, one {# unread mention} other {# unread mentions}}",
+ "tabChatWithMentionCount": "{tabLabel}, {displayCount} {count, plural, one {unread mention} other {unread mentions}}",
@@
- "tabMentionsWithMentionCount": "{tabLabel}, {count, plural, one {# unread mention} other {# unread mentions}}",
+ "tabMentionsWithMentionCount": "{tabLabel}, {displayCount} {count, plural, one {unread mention} other {unread mentions}}",
@@
- "mentionInboxBellAria": "{count, plural, one {# unread mention} other {# unread mentions}}",
+ "mentionInboxBellAria": "{displayCount} {count, plural, one {unread mention} other {unread mentions}}",Update callers to pass both count: unreadCount and displayCount: badgeLabel.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/i18n/src/messages/en.json` around lines 1629 - 1632, The ICU plural
messages tabChatWithMentionCount, tabMentionsWithMentionCount, and
mentionInboxBellAria must be split to use a numeric plural operand and a
separate display string; update the JSON entries to accept both count (numeric)
for the plural branch and displayCount for the visible/badge text (e.g.
"{tabLabel}, {count, plural, one {# unread mention} other {# unread mentions}}
({displayCount})" or similar), then update the callers
human-chat-panel-mention-inbox.tsx (where '99+' is passed),
human-chat-panel-tabs.tsx (where chatBadgeLabel and mentionBadgeLabel are
passed) to pass count: unreadCount (number) and displayCount: badgeLabel
(string) instead of passing the display string into the plural operand.
Add implementation-ready requirements for Matrix intentional mentions, mention highlights, tab/bell badges, mention side panel, notification centre entry, and per-room notification policy with phased commit plan. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Add MSC3952-style m.mentions.user_ids on send/edit paths, expose mentionedUserIds on Message, helpers for extracting MXIDs from plaintext, and unit tests for mention helpers. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Add @-token parsing, room member suggestions, keyboard navigation, and Matrix MXID insertion. Enable the At toolbar when members are available and pass joined members from the human chat panel. Add i18n for the mention list and empty state. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Map Matrix m.mentions into the panel message model and apply a Discord-style row tint for the mentioned viewer only. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Expose unreadMentionCount from matrix-chat-unread (highlight-only) and wire it to the Chat tab. Compute unread state even when Members is active so mention badges stay accurate. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Add a right Sheet listing recent m.mentions events with settings link to the aside notification centre. Place the bell in the chat header with an unread highlight badge and scroll the timeline when a row is chosen. Extend HumanChatPanelMessages with optional scroll-to-event targeting. Add i18n strings for the inbox. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Relax mergeMatrixMentionsIntoContent generics so Hypha/media payloads stay typed. Restore explicit casts where media bundle content widens union types beyond RoomMessageEventContent — fixes Deploy Preview check-types CI. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Clarify room-level badge semantics in thread mode; split policy matrix; fix MD029 list numbering; resolve §10 open points; update problem statement. Exclude current user from @ mention picker to match §10 decision. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Human chat and Matrix token API are on unless disabled via emergency kill switch (cookie, env, or Vercel toolbar). Remove enable-human-chat from Vercel flag discovery. Simplify CoherenceBlock and e2e tests. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Use Matrix RoomMember display resolution (including profile fallback) instead of falling back to raw MXIDs when member.name equals the user id. Shorten synthetic-looking localparts for secondary lines. Render mention rows with avatars, truncation, and clearer layout. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
The bell trigger used h-9 while sidebar toggle and chat action use h-7, stretching the header vertically on preview. Align dimensions and icon/badge scale with the existing header controls. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Replace the Sheet overlay and header bell with a Mentions tab next to Chat/Members, inline mention list panel, and notification centre link aligned to the right of the secondary nav. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
DHO notification centre lives under /[lang]/dho/[id]/[tab]/notification-centre; include the active tab from the path and align network/my-spaces branches with the profile menu. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Reintroduce h-7 bell with unread mention count beside the sidebar toggle; click switches to the Mentions tab without changing header height. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
… localparts Root cause: message body stores full MXIDs; the old mention pill regex split on the first colon, so bridged localparts (privy_did_...) were truncated. Formatted HTML also showed raw text. Parse MXIDs with the same pattern as the send path, resolve labels via room member state, and pass the same resolver into rich HTML text nodes. Export MATRIX_MXID_IN_PLAIN_TEXT; add test for localparts with colons. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
When the textarea is cleared, only setting height to auto can leave a stale scrollHeight in some browsers. Shrink to 0 then re-measure for empty content and reset scroll on the mirror backdrop. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
…rofiles Merge space roster (Person + matrix_user_links batch) with Matrix joined members so bridged-only participants appear and labels match Members. Treat Privy-style Matrix displaynames as technical and resolve picker rows via Person like the timeline. Adds batch Privy→MXID lookup server action and MentionCandidateRow component. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Subscribe to RoomStateEvent.Members and NewMember so mentionCandidates recomputes when a second user joins or leaves, enabling the @ control without a full page reload. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Enable the mention control only when another user is joined in the room, while keeping roster-merged suggestions for the dropdown. Uses the same membership epoch bump as candidate refresh. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
senderProfileLoading stayed true while senderPrivySub was set but usePersonBySub never returned a person, leaving the grey Skeleton bar with no text. Only show the skeleton during active link/person fetches; otherwise show Matrix fallback label. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Replace harsh red outline on flat grey with soft gradient glass, subtle border glow, inset highlight, and an upgraded REC dot with halo animation. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
On send, abort dictation and stop voice recording, discard in-flight voice attachment blob, and suppress SpeechRecognition finalize from repopulating the composer after parent clears input. Route all send triggers through sendMessage. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Reset backdrop + textarea heights with min-height cleared, reflow both layers, and run resize in useLayoutEffect when value updates so empty state paints at one-line height (fixes stuck tall box after dictation/async clear). Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
The textarea and URL backdrop were two grid cells; the row height was the max of both, so the shell stayed as tall as the last multiline state. Use a single-flow textarea with an absolute-positioned mirror; only the textarea sets block height. Simplifies autoResize (no backdrop height sync). Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
needsHyphaProfileForMatrixLabel used /^prev_privy_/ on the raw label; Matrix often sends @prev_privy_* so resolveSenderProfile stayed false and usePersonBySub never ran. Strip leading @ when matching bridged stems (shared helper with room labels). Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
…parts Detect bridged `prev_`/`prod_` display strings after `shortenMatrixIdForDisplay` (ellipsis instead of full underscore). Stop skipping those names when they equal the raw MXID so technical-name checks and profile resolution can run. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Normalize MXID extraction when a colon trails the homeserver; widen mention query validation for Unicode names; avoid duplicate @ and Enter hijacking in the composer; consume scroll targets only after a successful scroll; reuse roster labels for mention pills; sync FR-COMP-2 with roster merge; add tab ARIA labels and ICU plurals for bell/tab badges. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Timeline headers now prefer merged Hypha roster labels like mention pills. Unread mention badges use max(homeserver highlight, client counts from m.mentions + read cursor) when highlight stays zero. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
…bar check Collapse enable-human-chat toolbar override to a single undefined check. Document intentional default-on semantics vs generic feature-flag guidelines. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
- Normalize MXID captures after host:port before stripping sentence colons - Avoid email domains as @handle pills; prevent @@ in MXID pills - Parse mentions from latest m.replace content for local unread counts - Add enableHumanChat to Vercel flag discovery (default true, documented) - ICU plural tab aria strings for mention badges (all locales) Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Remove enable-coherence checks from the Coherence route and DHO tab shell so preview deployments can validate signals without production flag parity. Introduce enable-Space-Memory (HYPHA_ENABLE_SPACE_MEMORY / NEXT_PUBLIC_ENABLE_SPACE_MEMORY) and render Space Memory only when enabled. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
… guideline CodeRabbit: clarify rollback (HYPHA_DISABLE_HUMAN_CHAT) and that returning false by default would require an explicit product decision. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
alexprate
force-pushed
the
cursor/human-chat-mentions-specs-93f4
branch
from
April 19, 2026 15:49
880f0bd to
e3472db
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (8)
packages/epics/src/common/human-chat-panel/matrix-chat-unread.ts (1)
47-144:⚠️ Potential issue | 🟠 MajorBuild the replacement map once per unread-count pass.
countUnreadMentionMessagesForUseriterates the timeline, thenwireContentForMentionParsescans the same timeline per message. This keeps the unread mention path O(n²), and Line 139 computes it on every state derivation.⚡ Proposed fix
+function latestReplacementByRootId( + timeline: MatrixEvent[], +): Map<string, MatrixEvent> { + const latestByRootId = new Map<string, MatrixEvent>(); + + for (const cand of timeline) { + const rootId = getMessageReplaceTargetEventId(cand); + if (!rootId) continue; + if (isRedactedRoomMessageEvent(cand)) continue; + + const prev = latestByRootId.get(rootId); + if (!prev || cand.getTs() >= prev.getTs()) { + latestByRootId.set(rootId, cand); + } + } + + return latestByRootId; +} + /** Latest message content for mention parsing (`m.replace` edits target the root id). */ function wireContentForMentionParse( - room: Room, rootEvent: MatrixEvent, + replacementsByRootId: Map<string, MatrixEvent>, ): Record<string, unknown> | undefined { const rootId = rootEvent.getId(); if (!rootId) return undefined; - let latest = rootEvent; - let latestTs = rootEvent.getTs(); - - for (const cand of room.getLiveTimeline().getEvents()) { - if (getMessageReplaceTargetEventId(cand) !== rootId) continue; - if (isRedactedRoomMessageEvent(cand)) continue; - const ts = cand.getTs(); - if (ts >= latestTs) { - latestTs = ts; - latest = cand; - } - } + const latest = replacementsByRootId.get(rootId) ?? rootEvent; const content = latest.getContent(); return content && typeof content === 'object' @@ ): number { const timeline = room.getLiveTimeline().getEvents(); + const replacementsByRootId = latestReplacementByRootId(timeline); let n = 0; @@ const ids = parseMentionUserIdsFromWireContent( - wireContentForMentionParse(room, ev), + wireContentForMentionParse(ev, replacementsByRootId), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/human-chat-panel/matrix-chat-unread.ts` around lines 47 - 144, The unread-mention path is O(n²) because countUnreadMentionMessagesForUser calls wireContentForMentionParse which rescans the timeline per message; fix by building the message-replacement map once in computeHumanChatUnreadState (map from original/root event id to the latest non-redacted event content or its parsed wire content), then change countUnreadMentionMessagesForUser to accept that precomputed map (or a lookup function) instead of calling wireContentForMentionParse so each timeline event is inspected only once; update references to wireContentForMentionParse and the call at Line 139 to use the new precomputed map and ensure functions still handle missing/null entries.packages/i18n/src/messages/fr.json (1)
1637-1640:⚠️ Potential issue | 🟠 MajorKeep numeric plural selection separate from capped display text.
These ICU plural messages still depend on
countfor both grammar and display. If the UI passes capped labels like99+, plural selection breaks; use numericcountplus a separatedisplayCountparameter across locale counterparts and call sites.🌐 Proposed message shape
- "tabChatWithMentionCount": "{tabLabel}, {count, plural, one {# mention non lue} other {# mentions non lues}}", + "tabChatWithMentionCount": "{tabLabel}, {displayCount} {count, plural, one {mention non lue} other {mentions non lues}}", @@ - "tabMentionsWithMentionCount": "{tabLabel}, {count, plural, one {# mention non lue} other {# mentions non lues}}", + "tabMentionsWithMentionCount": "{tabLabel}, {displayCount} {count, plural, one {mention non lue} other {mentions non lues}}", @@ - "mentionInboxBellAria": "{count, plural, one {# mention non lue} other {# mentions non lues}}", + "mentionInboxBellAria": "{displayCount} {count, plural, one {mention non lue} other {mentions non lues}}",Also applies to: 1681-1681
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/i18n/src/messages/fr.json` around lines 1637 - 1640, The ICU plural messages tabChatWithMentionCount and tabMentionsWithMentionCount currently use the same `count` for plural selection and the displayed label, which breaks when the UI passes capped strings like "99+"; change the message shape to use `count` (numeric) for the plural selection and a separate `displayCount` (string) for the shown label, update these two keys' ICU patterns accordingly, and update all call sites to pass numeric `count` for plural logic and `displayCount` for rendering (ensuring other locale files follow the same parameter names).packages/i18n/src/messages/de.json (1)
1637-1640:⚠️ Potential issue | 🟠 MajorApply the same
count/displayCountsplit here.These German ICU plural strings have the same capped-label risk:
countshould stay numeric for plural selection, whiledisplayCountshould render values like99+.Also applies to: 1681-1681
docs/requirements/Features/human-chat-mentions/requirements.md (1)
108-108:⚠️ Potential issue | 🟡 MinorRemove the remaining notification-centre implementation ambiguity.
Line 248 resolves v1 to aside navigation, but Line 108 still allows embedding
NotificationCentreFormin the mention panel. Make FR-PANEL-4 require the aside route for v1 so implementers do not treat both as valid.📝 Proposed doc fix
-**FR-PANEL-4** Toolbar in panel: **Settings / gear** (or ellipsis) navigates to the existing **notification centre** experience (same **content** as `AsideNotificationCentrePage` / `NotificationCentreForm`), either by **routing to aside `notification-centre`** with close URL semantics mirroring `aside-notification-centre-page.tsx`, or by **embedding** the form inside the panel—implementation choice with preference for **reuse** of `NotificationCentreForm` and **one** canonical UX. +**FR-PANEL-4** Toolbar in panel: **Settings / gear** (or ellipsis) navigates to the existing **notification centre** experience (same **content** as `AsideNotificationCentrePage` / `NotificationCentreForm`) by **routing to aside `notification-centre`** with close URL semantics mirroring `aside-notification-centre-page.tsx`. Embedding the form inside the mention panel is non-v1 unless product reopens the decision in §10.Also applies to: 248-248
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/requirements/Features/human-chat-mentions/requirements.md` at line 108, Update FR-PANEL-4 to remove the embedding option and require the aside route for v1: change the text so the Settings/gear (or ellipsis) must navigate to the existing notification centre via routing to the aside "notification-centre" (mirroring aside-notification-centre-page.tsx / AsideNotificationCentrePage) and state that implementers must reuse NotificationCentreForm through that aside route rather than embedding it in the mention panel; ensure the wording explicitly declares one canonical UX and disallows embedding for v1.packages/feature-flags/src/index.ts (2)
64-70:⚠️ Potential issue | 🟡 MinorUse lowercase kebab-case for the Space Memory flag key.
enable-Space-Memoryis inconsistent with the other toolbar keys and makes exact-match overrides easy to misconfigure. Preferenable-space-memoryin both discovery metadata and runtime lookup.🛠️ Proposed fix
enableSpaceMemory: { - key: 'enable-Space-Memory', + key: 'enable-space-memory', defaultValue: false, description: 'Show the Space Memory panel on the Coherence tab', origin: 'hypha' as const, @@ export async function getEnableSpaceMemory(): Promise<boolean> { return getBooleanFlagFromToolbarCookieOrEnv( - 'enable-Space-Memory', + 'enable-space-memory', HYPHA_ENABLE_SPACE_MEMORY, process.env.NEXT_PUBLIC_ENABLE_SPACE_MEMORY, ); }As per coding guidelines,
packages/feature-flags/**should verify “Flags have clear naming conventions and documentation.”Also applies to: 169-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/feature-flags/src/index.ts` around lines 64 - 70, The feature flag definition enableSpaceMemory uses an inconsistent key 'enable-Space-Memory'; change its key to lowercase kebab-case 'enable-space-memory' in the enableSpaceMemory object (and make the identical change in the other occurrence around the second definition referenced) so discovery metadata and runtime lookups use 'enable-space-memory' consistently with other toolbar keys.
149-167:⚠️ Potential issue | 🟠 MajorMake the Human Chat kill switch non-overridable.
enable-human-chat=truefrom the toolbar currently returns beforeHYPHA_DISABLE_HUMAN_CHAT/NEXT_PUBLIC_DISABLE_HUMAN_CHAT, so a stale toolbar override can bypass the documented emergency rollback path.🛠️ Proposed fix
export async function getEnableHumanChat(): Promise<boolean> { + const store = await cookies(); + + if (store.get(HYPHA_DISABLE_HUMAN_CHAT)?.value === 'true') { + return false; + } + + if (process.env.NEXT_PUBLIC_DISABLE_HUMAN_CHAT === 'true') return false; + const overrides = await getVercelToolbarFlagOverrides(); const toolbarHumanChat = readBooleanOverride(overrides, 'enable-human-chat'); if (toolbarHumanChat !== undefined) return toolbarHumanChat; - const store = await cookies(); - - if (store.get(HYPHA_DISABLE_HUMAN_CHAT)?.value === 'true') { - return false; - } - const legacyEnable = store.get(HYPHA_ENABLE_HUMAN_CHAT)?.value; if (legacyEnable === 'false') return false; - if (process.env.NEXT_PUBLIC_DISABLE_HUMAN_CHAT === 'true') return false; if (process.env.NEXT_PUBLIC_ENABLE_HUMAN_CHAT === 'false') return false; return true; }As per coding guidelines,
packages/feature-flags/**should ensure default values and flag evaluation are safe and deterministic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/feature-flags/src/index.ts` around lines 149 - 167, The toolbar override returned by getVercelToolbarFlagOverrides/readBooleanOverride must not short-circuit the kill switch; change getEnableHumanChat so that it checks the non-overridable disable signals (cookie HYPHA_DISABLE_HUMAN_CHAT and env NEXT_PUBLIC_DISABLE_HUMAN_CHAT and legacy HYPHA_ENABLE_HUMAN_CHAT=false) before honoring a true toolbar override—i.e., evaluate and return false for any disable condition first, then consult toolbarHumanChat for an explicit true/false or fall back to the default true.packages/core/src/matrix/mentions.ts (1)
52-54:⚠️ Potential issue | 🟡 MinorValidate and dedupe wire mention IDs consistently.
isLikelyMatrixUserIdaccepts whitespace after the homeserver, andparseMentionUserIdsFromWireContentreturns unvalidated strings. Malformedm.mentions.user_idscan leak into UI/highlight logic.🛡️ Proposed fix
export function isLikelyMatrixUserId(id: string): boolean { - return /^@[^\s:]+:.+/.test(id); + return /^@[^\s:]+:[^\s]+$/.test(id); } @@ const ids = (raw as { user_ids?: unknown }).user_ids; if (!Array.isArray(ids)) return undefined; - const out = ids.filter((id): id is string => typeof id === 'string'); + const out = [ + ...new Set( + ids.filter( + (id): id is string => + typeof id === 'string' && isLikelyMatrixUserId(id), + ), + ), + ]; return out.length > 0 ? out : undefined; }Also applies to: 131-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/matrix/mentions.ts` around lines 52 - 54, The validation for Matrix user IDs is too permissive and parseMentionUserIdsFromWireContent returns unvalidated, potentially malformed IDs from m.mentions.user_ids; tighten isLikelyMatrixUserId to reject whitespace after the homeserver (e.g. use a regex like /^@[^\s:]+:[^\s]+$/) and update parseMentionUserIdsFromWireContent to: iterate m.mentions.user_ids, trim values, validate each with isLikelyMatrixUserId, collect only valid IDs into a Set to dedupe, and return the deduped array so only well-formed, unique IDs reach UI/highlight logic.packages/epics/src/common/human-chat-panel/matrix-room-member-display.ts (1)
16-21:⚠️ Potential issue | 🟠 MajorTreat all
prev_locals as synthetic, not onlyprev_privy.The PR objective calls out
prev_as a known synthetic prefix, but the detector and shortener still only matchprev_privy.prev_bridge_user-style labels will be trusted and shown.🐛 Proposed fix
function stemLooksLikeBridgedPrivyLocalpart(stem: string): boolean { const s = stem.trim(); - if (/^prev_privy/i.test(s) || /^prod_privy/i.test(s)) return true; - /** Production Matrix bridge locals often start with `prod_` (not always `prod_privy`). */ - if (/^prod_/i.test(s)) return true; + if (/^prev_/i.test(s) || /^prod_/i.test(s)) return true; return false; } @@ const looksSynthetic = - /privy|_did_|^prod_|^prev_privy|^prod_privy/i.test(local) || + /privy|_did_|^prod_|^prev_/i.test(local) || local.length > 28;Also applies to: 60-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/epics/src/common/human-chat-panel/matrix-room-member-display.ts` around lines 16 - 21, The detector function stemLooksLikeBridgedPrivyLocalpart currently only treats names starting with "prev_privy" as synthetic; change its logic to treat any localpart starting with "prev_" as synthetic (e.g., replace the /^prev_privy/i check with /^prev_/i or add an additional /^prev_/i test) so labels like "prev_bridge_user" are caught; apply the same change to the duplicate occurrence referenced at lines 60-62 to ensure all `prev_` locals are treated consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/`[lang]/dho/[id]/@tab/layout.tsx:
- Line 15: NavigationTabs is being forced to show the Coherence tab by passing a
bare coherenceEnabled prop; revert this to use the actual feature-flag value so
getEffectiveDhoTab() can guard disabled routes. Replace the hard-coded/coercive
prop on NavigationTabs in the layout component with the feature-flag boolean
(the same flag used elsewhere to determine Coherence availability) so
coherenceEnabled is true only when the feature flag indicates it should be;
ensure getEffectiveDhoTab() remains the single source-of-truth for routing
behavior.
In `@packages/core/src/matrix/server/actions.ts`:
- Around line 69-84: The current getMatrixUserIdsByPrivySubsAction exposes
Privy→Matrix mapping if any non-empty authToken is passed; to fix, validate and
authorize the token instead of presence-only: in
getMatrixUserIdsByPrivySubsAction verify the JWT (e.g., check signature and
required claims/roles/scopes) and reject requests without proper claims before
calling findMatrixUserIdsByPrivyUserIds, enforce a hard cap on privyUserIds
length (e.g., max N) and return a clear error if exceeded, and ensure this
action is only callable from a trusted server boundary (or document/rename to a
private/internal function) so arbitrary clients cannot enumerate mappings.
In
`@packages/epics/src/common/human-chat-panel/human-chat-mention-candidate-row.tsx`:
- Around line 60-76: The row is being disabled while profile/link metadata loads
(busy), making picks fail; change the disabled logic so the button remains
selectable when we have a mention identifier (matrixUserId or
matrixFallbackLabel). Update the disabled prop on the button (and any uses of
busy for interactivity) to use something like disabled = {!matrixUserId &&
!matrixFallbackLabel} while keeping busy only for visual state (opacity) so
onClick/onPick still works during loading; touch the const busy and the button's
disabled prop and preserve onClick/onMouseDown handlers.
In `@packages/epics/src/common/human-chat-panel/human-chat-mention-token.ts`:
- Around line 14-18: isAtWordStart currently only treats whitespace as a valid
preceding character, so mentions after punctuation fail; update the
isAtWordStart function to return true when atIndex <= 0 or when the previous
character is NOT an alphanumeric character (i.e., open on punctuation), for
example replacing /\s/.test(prev) with a negated alphanumeric check such as
!/[A-Za-z0-9]/.test(prev) or, for broader Unicode support,
!/[\p{L}\p{N}]/u.test(prev) so mentions like "(`@alice`" or ",`@alice`" correctly
open the picker.
In
`@packages/epics/src/common/human-chat-panel/human-chat-panel-mention-inbox.tsx`:
- Around line 37-50: The inbox loop must ignore Matrix edit replacement events
so they don't produce entries with non-scrollable eventIds; inside the for-loop
that iterates events (where variables ev, currentUserId,
parseMentionUserIdsFromWireContent, isRedactedRoomMessageEvent are used) add a
guard to skip events whose content indicates an edit replacement (e.g. if
ev.getContent()?.["m.relates_to"]?.rel_type === "m.replace") and continue the
loop before collecting the event into out.
In `@packages/epics/src/common/human-chat-panel/human-chat-panel-tabs.tsx`:
- Around line 34-46: The ICU plural messages tabChatWithMentionCount and
tabMentionsWithMentionCount are being given preformatted badge strings
(chatBadgeLabel/mentionBadgeLabel) which breaks pluralization; instead, keep
chatBadgeLabel/mentionBadgeLabel for UI display but pass the raw numeric counts
(e.g., chatMentionCount and mentionTabBadgeCount — or their capped numeric value
like 99 if you intend to cap the plural logic) as the count argument to
tabChatWithMentionCount and tabMentionsWithMentionCount. Locate usages of those
message calls and replace the badge-label argument with the underlying numeric
variables while leaving the label construction (chatBadgeLabel,
mentionBadgeLabel) unchanged for rendering.
In `@packages/epics/src/common/human-chat-panel/parse-simple-matrix-html.tsx`:
- Around line 119-124: The text-case always wraps node text in a <span> even
when no transform is needed; update the case in parse-simple-matrix-html.tsx so
that when transformText is provided you return <span
key={k}>{transformText(n.value)}</span>, and when transformText is falsy you
return the raw string n.value (no extra element or key) to preserve the original
DOM/sibling structure; locate the switch case handling 'text' and modify the
return accordingly.
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 1720-1725: The Mentions tab is using the Matrix-only resolver via
resolveMemberLabel which causes bridged users to show MXIDs; swap it to use the
roster-aware resolver by passing resolveMentionMemberLabel (or a thin wrapper
that delegates to resolveMentionMemberLabel) into the HumanChatPanelMentionTab's
resolveMemberLabel prop so the Mentions tab uses the roster override; update the
usage around HumanChatPanelMentionTab and ensure handleSelectMentionFromInbox
remains unchanged.
---
Duplicate comments:
In `@docs/requirements/Features/human-chat-mentions/requirements.md`:
- Line 108: Update FR-PANEL-4 to remove the embedding option and require the
aside route for v1: change the text so the Settings/gear (or ellipsis) must
navigate to the existing notification centre via routing to the aside
"notification-centre" (mirroring aside-notification-centre-page.tsx /
AsideNotificationCentrePage) and state that implementers must reuse
NotificationCentreForm through that aside route rather than embedding it in the
mention panel; ensure the wording explicitly declares one canonical UX and
disallows embedding for v1.
In `@packages/core/src/matrix/mentions.ts`:
- Around line 52-54: The validation for Matrix user IDs is too permissive and
parseMentionUserIdsFromWireContent returns unvalidated, potentially malformed
IDs from m.mentions.user_ids; tighten isLikelyMatrixUserId to reject whitespace
after the homeserver (e.g. use a regex like /^@[^\s:]+:[^\s]+$/) and update
parseMentionUserIdsFromWireContent to: iterate m.mentions.user_ids, trim values,
validate each with isLikelyMatrixUserId, collect only valid IDs into a Set to
dedupe, and return the deduped array so only well-formed, unique IDs reach
UI/highlight logic.
In `@packages/epics/src/common/human-chat-panel/matrix-chat-unread.ts`:
- Around line 47-144: The unread-mention path is O(n²) because
countUnreadMentionMessagesForUser calls wireContentForMentionParse which rescans
the timeline per message; fix by building the message-replacement map once in
computeHumanChatUnreadState (map from original/root event id to the latest
non-redacted event content or its parsed wire content), then change
countUnreadMentionMessagesForUser to accept that precomputed map (or a lookup
function) instead of calling wireContentForMentionParse so each timeline event
is inspected only once; update references to wireContentForMentionParse and the
call at Line 139 to use the new precomputed map and ensure functions still
handle missing/null entries.
In `@packages/epics/src/common/human-chat-panel/matrix-room-member-display.ts`:
- Around line 16-21: The detector function stemLooksLikeBridgedPrivyLocalpart
currently only treats names starting with "prev_privy" as synthetic; change its
logic to treat any localpart starting with "prev_" as synthetic (e.g., replace
the /^prev_privy/i check with /^prev_/i or add an additional /^prev_/i test) so
labels like "prev_bridge_user" are caught; apply the same change to the
duplicate occurrence referenced at lines 60-62 to ensure all `prev_` locals are
treated consistently.
In `@packages/feature-flags/src/index.ts`:
- Around line 64-70: The feature flag definition enableSpaceMemory uses an
inconsistent key 'enable-Space-Memory'; change its key to lowercase kebab-case
'enable-space-memory' in the enableSpaceMemory object (and make the identical
change in the other occurrence around the second definition referenced) so
discovery metadata and runtime lookups use 'enable-space-memory' consistently
with other toolbar keys.
- Around line 149-167: The toolbar override returned by
getVercelToolbarFlagOverrides/readBooleanOverride must not short-circuit the
kill switch; change getEnableHumanChat so that it checks the non-overridable
disable signals (cookie HYPHA_DISABLE_HUMAN_CHAT and env
NEXT_PUBLIC_DISABLE_HUMAN_CHAT and legacy HYPHA_ENABLE_HUMAN_CHAT=false) before
honoring a true toolbar override—i.e., evaluate and return false for any disable
condition first, then consult toolbarHumanChat for an explicit true/false or
fall back to the default true.
In `@packages/i18n/src/messages/fr.json`:
- Around line 1637-1640: The ICU plural messages tabChatWithMentionCount and
tabMentionsWithMentionCount currently use the same `count` for plural selection
and the displayed label, which breaks when the UI passes capped strings like
"99+"; change the message shape to use `count` (numeric) for the plural
selection and a separate `displayCount` (string) for the shown label, update
these two keys' ICU patterns accordingly, and update all call sites to pass
numeric `count` for plural logic and `displayCount` for rendering (ensuring
other locale files follow the same parameter names).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: eb191bd3-6d0c-48c0-9c29-c5d0d913b0e9
📒 Files selected for processing (45)
apps/web-e2e/src/human-chat-panel-avatar.spec.tsapps/web-e2e/src/human-chat-panel-feature-flag.spec.tsapps/web-e2e/src/human-chat-panel-header.spec.tsapps/web-e2e/src/human-chat-panel-members.spec.tsapps/web-e2e/src/human-chat-panel-resize.spec.tsapps/web-e2e/src/human-chat-panel-space-switch.spec.tsapps/web-e2e/src/menu-top-consistent-height.spec.tsapps/web-e2e/src/pages/human-chat-panel.page.tsapps/web-e2e/src/panel-layout.spec.tsapps/web-e2e/src/panels-space-context.spec.tsapps/web/src/app/[lang]/dho/[id]/@tab/coherence/page.tsxapps/web/src/app/[lang]/dho/[id]/@tab/layout.tsxdocs/requirements/Features/human-chat-mentions/requirements.mdpackages/cookie/src/constants.tspackages/core/src/matrix/__tests__/mentions.test.tspackages/core/src/matrix/client/hooks/index.tspackages/core/src/matrix/client/hooks/use-matrix-user-ids-by-privy-subs.tspackages/core/src/matrix/client/providers/matrix-provider.tsxpackages/core/src/matrix/edit-room-message-media-caption.tspackages/core/src/matrix/index.tspackages/core/src/matrix/mentions.tspackages/core/src/matrix/rich-reply.tspackages/core/src/matrix/server/actions.tspackages/core/src/matrix/server/queries.tspackages/core/src/matrix/types.tspackages/epics/src/coherence/components/coherence-block.tsxpackages/epics/src/common/human-chat-panel/human-chat-mention-candidate-row.tsxpackages/epics/src/common/human-chat-panel/human-chat-mention-token.tspackages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-header.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-mention-inbox.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-message-bubble.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsxpackages/epics/src/common/human-chat-panel/human-chat-panel-tabs.tsxpackages/epics/src/common/human-chat-panel/index.tspackages/epics/src/common/human-chat-panel/matrix-chat-unread.tspackages/epics/src/common/human-chat-panel/matrix-room-member-display.tspackages/epics/src/common/human-chat-panel/parse-simple-matrix-html.tsxpackages/epics/src/common/human-right-panel.tsxpackages/feature-flags/src/index.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
💤 Files with no reviewable changes (1)
- apps/web-e2e/src/human-chat-panel-space-switch.spec.ts
| lang={lang} | ||
| coherenceEnabled={coherenceEnabled} | ||
| /> | ||
| <NavigationTabs id={daoSlug} lang={lang} coherenceEnabled /> |
Contributor
There was a problem hiding this comment.
Do not force-enable the Coherence tab.
Line 15 passes coherenceEnabled={true}, so NavigationTabs always renders Coherence and getEffectiveDhoTab() no longer guards disabled Coherence routes. Restore the feature-flag value instead of using a bare prop.
🛡️ Proposed fix
import { Locale } from '@hypha-platform/i18n';
import { ReactNode } from 'react';
+import { getEnableCoherence } from '@hypha-platform/feature-flags';
import { NavigationTabs } from '../_components/navigation-tabs';
@@
}) {
const { id: daoSlug, lang } = await params;
+ const coherenceEnabled = await getEnableCoherence();
return (
<>
- <NavigationTabs id={daoSlug} lang={lang} coherenceEnabled />
+ <NavigationTabs
+ id={daoSlug}
+ lang={lang}
+ coherenceEnabled={coherenceEnabled}
+ />
{children}
</>
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/`[lang]/dho/[id]/@tab/layout.tsx at line 15, NavigationTabs
is being forced to show the Coherence tab by passing a bare coherenceEnabled
prop; revert this to use the actual feature-flag value so getEffectiveDhoTab()
can guard disabled routes. Replace the hard-coded/coercive prop on
NavigationTabs in the layout component with the feature-flag boolean (the same
flag used elsewhere to determine Coherence availability) so coherenceEnabled is
true only when the feature flag indicates it should be; ensure
getEffectiveDhoTab() remains the single source-of-truth for routing behavior.
Comment on lines
+69
to
+84
| /** Batch map Privy subs → Matrix MXIDs for the mention picker (space roster merge). */ | ||
| export async function getMatrixUserIdsByPrivySubsAction( | ||
| { | ||
| privyUserIds, | ||
| environment, | ||
| }: { | ||
| privyUserIds: string[]; | ||
| environment: Environment; | ||
| }, | ||
| { authToken }: { authToken?: string } = {}, | ||
| ): Promise<Array<{ privyUserId: string; matrixUserId: string }>> { | ||
| if (!authToken) { | ||
| throw new Error('authToken is required for matrix user id batch lookup'); | ||
| } | ||
| return findMatrixUserIdsByPrivyUserIds({ privyUserIds, environment }, { db }); | ||
| } |
Contributor
There was a problem hiding this comment.
Do not expose this lookup with only an auth-token presence check.
getMatrixUserIdsByPrivySubsAction is consumed from client code, but any caller can pass a non-empty authToken and enumerate Privy→Matrix mappings for arbitrary IDs/environments. Put this behind a verified API/server boundary, or validate the JWT/claims before querying; also cap privyUserIds to a small batch size. Based on learnings, server actions in this file previously used presence-only authToken because they were exclusively called from the pre-authenticated /api/matrix/token route.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/matrix/server/actions.ts` around lines 69 - 84, The current
getMatrixUserIdsByPrivySubsAction exposes Privy→Matrix mapping if any non-empty
authToken is passed; to fix, validate and authorize the token instead of
presence-only: in getMatrixUserIdsByPrivySubsAction verify the JWT (e.g., check
signature and required claims/roles/scopes) and reject requests without proper
claims before calling findMatrixUserIdsByPrivyUserIds, enforce a hard cap on
privyUserIds length (e.g., max N) and return a clear error if exceeded, and
ensure this action is only callable from a trusted server boundary (or
document/rename to a private/internal function) so arbitrary clients cannot
enumerate mappings.
Comment on lines
+60
to
+76
| const busy = | ||
| (!privySub && loadingLink) || (Boolean(resolvedSub) && loadingPerson); | ||
|
|
||
| return ( | ||
| <button | ||
| type="button" | ||
| role="option" | ||
| aria-selected={isActive} | ||
| title={matrixUserId} | ||
| disabled={busy} | ||
| className={cn( | ||
| 'flex w-full min-w-0 items-center gap-2.5 rounded-sm px-2 py-1.5 text-left text-sm', | ||
| busy && 'opacity-70', | ||
| isActive ? 'bg-muted text-foreground' : 'hover:bg-muted/80', | ||
| )} | ||
| onMouseDown={(ev) => ev.preventDefault()} | ||
| onClick={onPick} |
Contributor
There was a problem hiding this comment.
Keep mention rows selectable while profile metadata loads.
matrixUserId and matrixFallbackLabel are enough to insert the mention. Disabling the row while profile/link data loads makes the picker unresponsive on slow or stalled lookups.
🐛 Proposed fix
aria-selected={isActive}
title={matrixUserId}
- disabled={busy}
+ aria-busy={busy || undefined}
className={cn(
'flex w-full min-w-0 items-center gap-2.5 rounded-sm px-2 py-1.5 text-left text-sm',
busy && 'opacity-70',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/epics/src/common/human-chat-panel/human-chat-mention-candidate-row.tsx`
around lines 60 - 76, The row is being disabled while profile/link metadata
loads (busy), making picks fail; change the disabled logic so the button remains
selectable when we have a mention identifier (matrixUserId or
matrixFallbackLabel). Update the disabled prop on the button (and any uses of
busy for interactivity) to use something like disabled = {!matrixUserId &&
!matrixFallbackLabel} while keeping busy only for visual state (opacity) so
onClick/onPick still works during loading; touch the const busy and the button's
disabled prop and preserve onClick/onMouseDown handlers.
Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Tighten MXID validation; dedupe wire user_ids; O(n) replacement map for unread mention counts; skip m.replace rows in mention inbox; roster resolver for inbox; ICU counts for tab aria-labels; mention token after punctuation; lowercase Space Memory toolbar key; Fragment-only rich text when no transform. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
Use translucent accent-9 chips with inset ring (matches button hue); tint @mention rows with accent scale instead of amber so pills harmonize on highlight. Co-authored-by: webguru-hypha <webguru-hypha@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@ mention list: why 1 user + wrong name (fix)
Root cause (two issues):
useMembers→Person.name/surname. The@picker only usedroom.getJoinedMembers(). MatrixRoomMember.namefor bridged accounts is often the same technical localpart asprev_privy_*, so we showed that string as the “display” name.matrixMemberDisplayLabeltreatedmember.name !== userIdas human-readable—butnamecan equal the ugly localpart (prev_privy_…), not the full@…:homeserver, so we incorrectly trusted it.Fix:
looksLikeTechnicalMatrixDisplayName) — ignoreprev_/prod_/privy_did_privyMatrix display names (same issue class as timeline headers).person.sub → matrix MXIDvia newmatrix_user_linksquery +getMatrixUserIdsByPrivySubsAction, same env as Matrix. Anyone on the Members list with a link appears in@too (not only Matrix join).HumanChatMentionCandidateRow: resolveusePersonBySubwhen we knowprivySubfrom roster (no extra Matrix→Privy round-trip), elseuseUserPrivyIdByMatrixId— same pipeline as chat bubbles.Commit:
fix(chat): align @ mention picker with Members tab roster and Hypha profilesSummary by CodeRabbit