Skip to content

feat: improve chat mention notifications and consent controls#2255

Open
webguru-hypha wants to merge 82 commits into
mainfrom
spec/chat-mention-notification-overhaul
Open

feat: improve chat mention notifications and consent controls#2255
webguru-hypha wants to merge 82 commits into
mainfrom
spec/chat-mention-notification-overhaul

Conversation

@webguru-hypha

@webguru-hypha webguru-hypha commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add OneSignal expert role definitions and register them in the agent router
  • add a Discord-parity chat mentions/notifications requirements spec for implementation planning
  • implement Notification Centre mention-notification consent (model, validation, UI, persistence) and localize the new setting across supported locales

Test plan

  • Verify Notification Centre renders mention consent Yes/No control
  • Verify mention consent persists through saveConfigurations and reloads from OneSignal tags
  • Verify new i18n key exists in en, de, fr, es, and pt
  • Validate end-to-end mention-triggered push/email suppression when consent is disabled (follow-up integration test)

Made with Cursor

Summary by CodeRabbit

  • New Features
    • Mention notifications: opt-in/out consent for desktop/mobile/email, inbox navigation, deep-link handling with graceful fallbacks and missing-message notice.
    • Signal team scoped collaboration: thread-scoped access controls, request-to-join, read-only composer mode, and member management UI.
    • Coherence boards: create/manage/delete boards and archive signals; preserved board filters when creating items.
  • Localization
    • Updated English, German, Spanish, French, and Portuguese UI strings (mention notifications, Signal Team flows, boards, onboarding subtitle).

Review Change Stack

Add a dedicated mention-notification consent setting to Notification Centre, persist it to OneSignal tags, and localize the new control across all supported locales. Also introduce OneSignal expert role definitions and a ready-to-implement chat/notification specification for Discord-parity mention flows.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR implements a Discord-parity mention notification system: specification, consent tag and UI, OneSignal integration, client/server notification plumbing (notifyChatMention), Matrix sendMessage eventId extraction for deep-links, deep-link retry/fallback, signal-team authorization and composer locking, coherence boards UI, i18n updates, and OneSignal agent roles.

Changes

Mention Notification System with Consent and Signal-Team Support

Layer / File(s) Summary
Feature specification and requirements
docs/requirements/Features/chat-mentions-notifications-discord-parity/requirements.md
Comprehensive spec: mention behavior, unread lifecycle, multi-channel OneSignal/email delivery with idempotency, deep-link contract, observability, and signal-team extension with acceptance criteria.
Consent tag constant and OneSignal integration
packages/notifications/src/constants/tags.ts, packages/notifications/src/hooks/use-notifications.ts
Adds TAG_MENTION_CONSENT and extends notifications hook to read/write consent from OneSignal user tags.
Consent form UI, validation schema, and localization
packages/epics/src/notifications/hooks/validation.ts, packages/epics/src/notifications/components/notification-centre-form.tsx, packages/i18n/src/messages/*.json
Adds mentionNotificationsConsent to schema and form UI with yes/no control; adds translation keys and mention fallback strings across locales.
Matrix sendMessage event ID extraction
packages/core/src/matrix/client/providers/matrix-provider.tsx
Introduces SendMessageResult, helper to extract event_id, and returns eventId from all send flows for deep-link construction.
Client notification input type and hook extension
packages/core/src/notifications/client/hooks/use-send-notifications.ts, packages/notifications/src/hooks/use-send-notifications.ts
Defines NotifyChatMentionInput and exposes notifyChatMention SWR mutation trigger in notification hooks (auth-gated).
Notification utilities and tests
packages/notifications/src/actions/notify-chat-mention.utils.ts, packages/notifications/src/actions/notify-chat-mention.test.ts
Adds sanitizeMentionIds, HTML-escape, URL validation, and buildMentionEmailBody plus tests validating sanitization, escaping, and safe URL fallback.
Server-side chat mention notification action
packages/notifications/src/actions/notify-chat-mention.ts, packages/notifications/src/actions/index.ts
Implements notifyChatMentionAction server action: auth validation, recipient DB lookup, payload construction with required consent tag, concurrent push/email dispatch, partial-failure logging, and export.
Message list deep-link retry and fallback callback
packages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsx
Adds onScrollTargetNotFound and retry tracking to invoke fallback UI when a deep-linked event cannot be located.
Signal-team state parsing and coherence helpers
packages/epics/src/common/human-right-panel.tsx
Parses timeline marker events to derive signal-team membership/pending lists and adds coherence-session persistence helpers for coherence navigation context.
Mention dispatch, signal-team filtering, and navigation notice handling
packages/epics/src/common/human-right-panel.tsx
Captures sendMessage eventId, filters mention targets per signal-team policy, constructs canonical ?msg= deep-links, dispatches notifyChatMention on send success when appropriate, and manages navigation/fallback notices.
Signal-team authorization gating and member management UI
packages/epics/src/common/human-right-panel.tsx
Computes policy, gates mention candidates and send/call actions, provides manage-members panel with pending approvals, and syncs membership via timeline re-derivation.
Composer locked mode for signal-team restricted access
packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx
Adds composerLocked/composerLockedMessage props; disables mentions, sending, attachments, emoji, voice, and dictation while locked and closes popovers on lock.
Coherence: boards, archiving, grid, and block wiring
packages/epics/src/coherence/components/signal-section.tsx, signal-card.tsx, signal-grid.tsx, coherence-block.tsx
Adds persisted signal boards (create/select/delete), hideArchived toggle from CoherenceBlock, archive (not delete) behavior for SignalCard, and grid/style tweaks.
New signal page query parsing
apps/web/src/app/[lang]/dho/[id]/@aside/[tab]/new-signal/page.tsx
Parses boardTag search params, normalizes/filters against COHERENCE_TAGS, and passes inherited tags into CreateSignalForm initial values.
Core server exports
packages/core/src/server.ts
Re-exports coherence tags module.
AI agent roles for OneSignal expertise
.agents/AGENTS.md, .agents/roles/senior-onesignal-notifications-engineer.base.md, .agents/roles/senior-onesignal-expert.base.md
Registers OneSignal-focused agent roles and alias with detailed role definitions and playbook.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • hypha-dao/hypha-web#1821: Related changes to the notifications client hook API; both PRs touch use-send-notifications types and hooks.
  • hypha-dao/hypha-web#1373: Prior work on notifications/OneSignal backend that this PR extends with notify-chat-mention and consent tagging.
  • hypha-dao/hypha-web#2147: Related composer/toolbar changes; overlaps with HumanChatPanelChatBar updates (mic/attach controls and disabled mention behavior).

Suggested labels

enhancement

Suggested reviewers

  • evgenibir
  • sergey3bv
  • plitzenberger
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows conventional commits format with type 'feat' and a clear, descriptive scope covering the main changes: chat mention notifications and consent controls.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch spec/chat-mention-notification-overhaul

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented May 19, 2026

Copy link
Copy Markdown

🔗 Custom preview URL: https://pr-2255.preview-app.hypha.earth

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/epics/src/notifications/components/notification-centre-form.tsx (1)

148-150: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Replace any in invalid-submit handler with a typed error shape.

Use FieldErrors<NotificationCentreFormValues> (or SubmitErrorHandler<NotificationCentreFormValues>) instead of any.

💡 Suggested fix
 import { useFieldArray, useForm } from 'react-hook-form';
+import { FieldErrors } from 'react-hook-form';
@@
-  const handleInvalid = async (err?: any) => {
+  const handleInvalid = async (
+    err?: FieldErrors<NotificationCentreFormValues>,
+  ) => {
     console.warn('Notification settings errors:', err);
   };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/epics/src/notifications/components/notification-centre-form.tsx`
around lines 148 - 150, The invalid-submit handler handleInvalid currently types
its parameter as any; change its signature to use the proper form error type
(e.g. FieldErrors<NotificationCentreFormValues> or
SubmitErrorHandler<NotificationCentreFormValues>) and update the parameter
name/type accordingly so the handler is strongly typed; locate handleInvalid in
notification-centre-form.tsx and replace the any with
FieldErrors<NotificationCentreFormValues> (or use
SubmitErrorHandler<NotificationCentreFormValues> for the whole handler type) and
adjust the console log or error handling to accept that typed shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/roles/senior-onesignal-notifications-engineer.base.md:
- Around line 18-19: The ordered list after the section heading incorrectly
continues numbering (shows "3." and "4."); change those list items for
"Requirements Engineering" and "Information Synthesis" to restart at "1." and
"2." respectively and ensure there's a blank line between the heading and the
list so the markdown restarts numbering correctly (edit the lines containing
"Requirements Engineering" and "Information Synthesis" in the
.agents/roles/senior-onesignal-notifications-engineer.base.md file).

In `@packages/epics/src/notifications/components/notification-centre-form.tsx`:
- Around line 300-305: The UI shows a hardcoded fallback string for the
mention-consent label; remove the literal and use the translation key only. In
notification-centre-form.tsx replace the conditional that renders FormLabel with
the fallback ('Mention Notifications Consent') so that it always calls
tNotificationCentre('channels.mentionConsent') (remove the ternary/has check) to
ensure all user-facing text uses the i18n key via tNotificationCentre and the
channels.mentionConsent identifier.

---

Outside diff comments:
In `@packages/epics/src/notifications/components/notification-centre-form.tsx`:
- Around line 148-150: The invalid-submit handler handleInvalid currently types
its parameter as any; change its signature to use the proper form error type
(e.g. FieldErrors<NotificationCentreFormValues> or
SubmitErrorHandler<NotificationCentreFormValues>) and update the parameter
name/type accordingly so the handler is strongly typed; locate handleInvalid in
notification-centre-form.tsx and replace the any with
FieldErrors<NotificationCentreFormValues> (or use
SubmitErrorHandler<NotificationCentreFormValues> for the whole handler type) and
adjust the console log or error handling to accept that typed shape.
🪄 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: 2e1f5941-6351-4ad8-9e0b-f940228ad129

📥 Commits

Reviewing files that changed from the base of the PR and between b15b71c and 9c76b4e.

📒 Files selected for processing (13)
  • .agents/AGENTS.md
  • .agents/roles/senior-onesignal-expert.base.md
  • .agents/roles/senior-onesignal-notifications-engineer.base.md
  • docs/requirements/Features/chat-mentions-notifications-discord-parity/requirements.md
  • packages/epics/src/notifications/components/notification-centre-form.tsx
  • packages/epics/src/notifications/hooks/validation.ts
  • packages/i18n/src/messages/de.json
  • packages/i18n/src/messages/en.json
  • packages/i18n/src/messages/es.json
  • packages/i18n/src/messages/fr.json
  • packages/i18n/src/messages/pt.json
  • packages/notifications/src/constants/tags.ts
  • packages/notifications/src/hooks/use-notifications.ts

Comment thread .agents/roles/senior-onesignal-notifications-engineer.base.md Outdated
Comment thread packages/epics/src/notifications/components/notification-centre-form.tsx Outdated
Alex Prate and others added 4 commits May 19, 2026 13:31
Add chat mention notification dispatch that maps mentioned Matrix users to Hypha recipients, enforces mention consent tags, and sends push/email notifications with deep links to the mentioned message. Also return sent event ids from Matrix sendMessage and wire client notification hooks for chat mentions.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add resilient mention navigation fallback when a target message or room cannot be resolved, with non-blocking user notices and localized copy. Also extract and test mention notification helper utilities to improve confidence in mention recipient normalization and email body rendering.

Co-authored-by: Cursor <cursoragent@cursor.com>
Reset supporting competencies numbering to restart at the section boundary so markdown lint rules pass and list formatting remains consistent.

Co-authored-by: Cursor <cursoragent@cursor.com>
Use the translation key directly for the mention consent form label so notification-center UI strings remain fully i18n-driven.

Co-authored-by: Cursor <cursoragent@cursor.com>
@webguru-hypha

Copy link
Copy Markdown
Contributor Author

Autofix Review Completed

Resolved all currently unresolved CodeRabbit comments with one commit per comment.

Commits

  • a2b8829b7 — restart ordered list numbering in OneSignal role markdown
  • 9805981a2 — remove hardcoded mention-consent fallback label and use i18n key only

Branch is clean and fully pushed.

Alex Prate and others added 3 commits May 19, 2026 13:58
Format the mention notification and chat panel files to satisfy the format:check workflow.

Co-authored-by: Cursor <cursoragent@cursor.com>
Allow @ mentions in coherence/signal-linked threads by merging candidates from the thread room and the parent space chat room membership.

Co-authored-by: Cursor <cursoragent@cursor.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/epics/src/notifications/components/notification-centre-form.tsx (1)

148-150: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Remove any type from invalid-submit handler and use proper form error typing.

The err?: any parameter weakens type safety. Use SubmitErrorHandler<NotificationCentreFormValues> (the standard React Hook Form pattern already used elsewhere in the codebase, e.g., people-redeem-form.tsx).

♻️ Proposed fix
-import { useFieldArray, useForm } from 'react-hook-form';
+import {
+  SubmitErrorHandler,
+  useFieldArray,
+  useForm,
+} from 'react-hook-form';
...
-  const handleInvalid = async (err?: any) => {
-    console.warn('Notification settings errors:', err);
-  };
+  const handleInvalid: SubmitErrorHandler<NotificationCentreFormValues> =
+    async (errors) => {
+      console.warn('Notification settings errors:', errors);
+    };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/epics/src/notifications/components/notification-centre-form.tsx`
around lines 148 - 150, The invalid-submit handler handleInvalid currently types
its parameter as err?: any which reduces type safety; change its signature to
use React Hook Form's SubmitErrorHandler<NotificationCentreFormValues> (i.e.
const handleInvalid: SubmitErrorHandler<NotificationCentreFormValues> = async
(errors) => { ... }) and update the parameter name from err to errors, remove
console.warn any-typing, and use the typed errors inside the body; add/import
SubmitErrorHandler and NotificationCentreFormValues if not already imported.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 964-967: The memo that builds candidateRoomIds (using
candidateRoomIds, roomId, mode, space.chatRoomId) only causes
mentionMembershipEpoch to update for the active room; ensure changes to the
parent space room also refresh mentions by including space.chatRoomId (trimmed)
in the dependency logic that computes candidateRoomIds and in the place that
bumps mentionMembershipEpoch. Specifically, update the memo/effect that uses
candidateRoomIds so it depends on space.chatRoomId?.trim() (and mode/roomId as
already present) or compute a stable key from the Set and use that to trigger
the mentionMembershipEpoch increment whenever the parent space chatRoomId
changes.
- Around line 2351-2362: The deep link is built using the current pathname
(variable pathname) which may be outside the target space; replace pathname with
the canonical route that mounts the target space/chat before appending the
msg/chat query params. Locate the deep link construction (uses pathname, roomId,
sendResult.eventId) and call or compute the canonical space route for the target
room (e.g., getCanonicalSpaceRoute(roomId) or derive the space route from
roomId/spaceId) and use that route instead of pathname when composing
`${window.location.origin}${...}` so the resulting link mounts the correct space
and the msg/chat params resolve.

In `@packages/notifications/src/actions/notify-chat-mention.ts`:
- Around line 61-79: The Promise.allSettled call in notify-chat-mention.ts
currently discards results so failed push or email deliveries are swallowed;
change the code to capture the results (e.g., const results = await
Promise.allSettled([...]) ), inspect each result for status === "rejected" for
sendPushNotifications and sendEmailNotifications, log the rejection details
(error messages/stack) via the existing logger/processLogger, and if both
channels failed throw or return an error to surface the failure; ensure you
reference the sendPushNotifications and sendEmailNotifications calls and
preserve the built email body via buildMentionEmailBody while adding this error
handling and logging logic.

In `@packages/notifications/src/actions/notify-chat-mention.utils.ts`:
- Around line 14-23: The template currently interpolates url directly into the
anchor href (variable url), allowing attribute injection and unsafe schemes;
validate the url scheme (allow only http or https) and reject or replace unsafe
values with a safe fallback, then HTML-escape it for attribute context (escape
quotes, ampersand, <, > — e.g., produce escapedUrl) before interpolation; update
the template returned by the function that produces the mention HTML to use
escapedUrl instead of url and preserve existing escapedActor and escapedPreview
logic so the href cannot be exploited.

---

Outside diff comments:
In `@packages/epics/src/notifications/components/notification-centre-form.tsx`:
- Around line 148-150: The invalid-submit handler handleInvalid currently types
its parameter as err?: any which reduces type safety; change its signature to
use React Hook Form's SubmitErrorHandler<NotificationCentreFormValues> (i.e.
const handleInvalid: SubmitErrorHandler<NotificationCentreFormValues> = async
(errors) => { ... }) and update the parameter name from err to errors, remove
console.warn any-typing, and use the typed errors inside the body; add/import
SubmitErrorHandler and NotificationCentreFormValues if not already imported.
🪄 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: 1c979d89-0908-4088-8621-988ed0113442

📥 Commits

Reviewing files that changed from the base of the PR and between 9c76b4e and cd7a952.

📒 Files selected for processing (16)
  • .agents/roles/senior-onesignal-notifications-engineer.base.md
  • packages/core/src/matrix/client/providers/matrix-provider.tsx
  • packages/core/src/notifications/client/hooks/use-send-notifications.ts
  • packages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsx
  • packages/epics/src/common/human-right-panel.tsx
  • packages/epics/src/notifications/components/notification-centre-form.tsx
  • packages/i18n/src/messages/de.json
  • packages/i18n/src/messages/en.json
  • packages/i18n/src/messages/es.json
  • packages/i18n/src/messages/fr.json
  • packages/i18n/src/messages/pt.json
  • packages/notifications/src/actions/index.ts
  • packages/notifications/src/actions/notify-chat-mention.test.ts
  • packages/notifications/src/actions/notify-chat-mention.ts
  • packages/notifications/src/actions/notify-chat-mention.utils.ts
  • packages/notifications/src/hooks/use-send-notifications.ts

Comment thread packages/epics/src/common/human-right-panel.tsx Outdated
Comment thread packages/epics/src/common/human-right-panel.tsx Outdated
Comment thread packages/notifications/src/actions/notify-chat-mention.ts Outdated
Comment thread packages/notifications/src/actions/notify-chat-mention.utils.ts Outdated
@webguru-hypha

Copy link
Copy Markdown
Contributor Author

Autofix Review Completed

Applied 4 CodeRabbit autofix comments with one commit per comment.

Commits

  • b091ba408 — refresh mention candidates when parent space room membership changes
  • 9b7ad8dd2 — build mention deep links from canonical space route
  • 16b6350d6 — surface mention delivery channel failures from Promise.allSettled
  • 96ed7a8c0 — sanitize mention email links and escape href attributes

Validation run

  • pnpm turbo run check-types --filter=@hypha-platform/epics
  • pnpm exec vitest run .worktrees/spec-chat-mention-notification-overhaul/packages/notifications/src/actions/notify-chat-mention.test.ts

CI is currently running on the latest commits.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 15

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@docs/requirements/Features/chat-mentions-notifications-discord-parity/requirements.md`:
- Around line 199-203: In
docs/requirements/Features/chat-mentions-notifications-discord-parity/requirements.md
the ordered lists that currently begin with "6. As a signal owner/editor..." and
"9. As a non-team member..." (and the similar block at 325-338) must be
normalized to satisfy markdownlint MD029: restart each logical ordered-list
block at 1 (or make them consistently sequential within each block) so change
the leading numerals (e.g., 6., 7., 8., 9., 10.) to 1., 2., 3., etc. for each
separate list block and verify other list blocks in the file follow the same
convention.

In `@packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx`:
- Around line 484-490: The component HumanChatPanelChatBar allows drag-and-drop
to call pushDrafts even when composerLocked is true; update all drop/attachment
ingestion paths to early-return or no-op when composerLocked is true (e.g.,
inside the onDrop, handleDrop, or addAttachment handlers that call pushDrafts)
so draft ingestion is guarded by if (!composerLocked) before calling pushDrafts;
apply the same guard to the other affected blocks where pushDrafts is invoked so
read-only/locked mode cannot mutate drafts.
- Around line 489-490: The combined condition atMentionInteractable is causing
locked composers to show the "no members loaded" tooltip; separate the
membership availability check from the lock check: compute a membershipAvailable
boolean from (mentionPickerEnabled ?? mentionCandidates.length > 0), then set
atMentionInteractable = !composerLocked && membershipAvailable; update the
tooltip logic to show a lock-specific message when composerLocked is true and
only show the "no members loaded" message when membershipAvailable is false and
composerUnlocked; apply the same split/tooltip change to the other occurrence
around the mention logic referenced (lines ~2059-2061).

In `@packages/epics/src/common/human-right-panel.tsx`:
- Around line 2733-2752: The hook that builds the mention deep link uses
spaceSlug (used to compute canonicalSpaceSlug at the mention deep-link logic
around the canonicalSpaceSlug usage) but spaceSlug is missing from the
dependency array for the effect/memo; add spaceSlug to the dependency array
alongside the existing entries (input, roomId, ..., t) so the effect
recalculates canonicalSpaceSlug and the notification deep link whenever
spaceSlug changes, preventing stale closures.
- Around line 1268-1274: The event content object passed to client.sendEvent
contains custom fields (coherenceSlug, memberMatrixUserIds, updatedAt) not
present on the Matrix SDK's RoomMessage content type; update the call to either
cast the payload to a permissive type or create and use a custom content
interface (e.g., SignalTeamEventContent extends IContent with coherenceSlug?:
string|null, memberMatrixUserIds: string[], updatedAt: string, plus body and
msgtype) and use that type for client.sendEvent<SignalTeamEventContent>(...) for
this call and the other signal-team sends (the nearby sendEvent calls referenced
in the comment) so TypeScript accepts the custom properties.
- Around line 1283-1301: requestSignalTeamAccess currently swallows failures
because it uses try/finally without a catch; update it to add a catch block
around the client.sendEvent call that logs the error (use the project's logger
or console.error) and surfaces feedback to the user (e.g., set a local error
state like setSignalTeamError or call the app's toast/notification helper) so
failures aren't silent, while keeping the existing success path that calls
setSignalTeamPendingRequesterIds only when sendEvent succeeds and preserving
setSignalTeamBusy(false) in finally.
- Around line 1303-1346: approveSignalTeamRequester currently lacks error
handling so failures are silently swallowed; add a catch block around the await
Promise.all call to capture errors, log them (via console.error or the app
logger) and surface the failure to the UI using the existing error/loading
mechanisms (e.g., call a state setter such as setSignalTeamError or dispatch a
user-facing toast), then rethrow or handle accordingly; keep the existing
finally that calls setSignalTeamBusy(false). Reference: function
approveSignalTeamRequester, Promise.all call, setSignalTeamBusy, and
setSignalTeamPendingRequesterIds.
- Around line 1264-1281: publishSignalTeamMembers currently awaits
client.sendEvent without error handling, causing unhandled promise rejections
when callers invoke it with void; wrap the body of publishSignalTeamMembers in a
try/catch, catch any error from client.sendEvent (and normalizeMatrixUserIds if
needed), log the error via the same logger used elsewhere (or processLogger)
with context ("publishSignalTeamMembers") and ensure you only call
setSignalTeamMemberIds and setSignalTeamPendingRequesterIds on success (or
revert state on failure) so state updates are not lost when the sendEvent fails.

In `@packages/epics/src/notifications/components/notification-centre-form.tsx`:
- Around line 93-95: The code calls
getSwitch(configuration.mentionNotificationsConsent) without guarding against
configs that lack that property, which causes missing values to be coerced to
'no'; change the call to first check whether configuration has an explicit
mentionNotificationsConsent and if not pass the intended default 'yes' into
getSwitch (e.g. use a null/undefined check or hasOwnProperty/'in' on
configuration before calling getSwitch), and apply the same guard to the other
occurrence of mentionNotificationsConsent conversion in the file so legacy
configs default to 'yes'.

In `@packages/i18n/src/messages/de.json`:
- Around line 1998-2010: The German locale file contains untranslated English
values for keys like composerLockedPlaceholder, signalTeamInteractionRestricted,
signalTeamBannerReadOnly, signalTeamRequestAccess, signalTeamRequestPending,
signalTeamManageTitle, signalTeamManageOpen, signalTeamManageClose,
signalTeamMemberListHint, signalTeamAddMember, signalTeamRemoveMember,
signalTeamPendingRequests, and signalTeamApproveRequester; replace those English
strings with proper German translations (keeping message intent and placeholders
intact) so de.json matches other locales' semantics and message keys (verify
spelling/formatting match other locale files and update any pluralization or
punctuation to follow German conventions).

In `@packages/i18n/src/messages/en.json`:
- Around line 179-181: Update the "title" value for the "mentionOnChatMessage"
message key to use natural English by replacing "You are mentioned on a chat
message" with "You are mentioned in a chat message"; locate the JSON object
named mentionOnChatMessage and modify its title property accordingly so the
description stays unchanged.

In `@packages/i18n/src/messages/es.json`:
- Around line 1990-2002: The Spanish locale contains English literals for the
new Signal Team messages; update the translations in the es.json entries for
keys composerLockedPlaceholder, signalTeamInteractionRestricted,
signalTeamBannerReadOnly, signalTeamRequestAccess, signalTeamRequestPending,
signalTeamManageTitle, signalTeamManageOpen, signalTeamManageClose,
signalTeamMemberListHint, signalTeamAddMember, signalTeamRemoveMember,
signalTeamPendingRequests, and signalTeamApproveRequester with proper Spanish
strings that match the tone and punctuation of the existing locale (e.g., "Only
selected team members can interact in this thread." → "Solo los miembros
seleccionados del equipo pueden participar en este hilo."). Ensure
pluralization/verb forms and capitalization follow other messages in es.json and
keep JSON syntax unchanged.

In `@packages/i18n/src/messages/fr.json`:
- Around line 1990-2003: The listed i18n keys (composerLockedPlaceholder,
signalTeamInteractionRestricted, signalTeamBannerReadOnly,
signalTeamRequestAccess, signalTeamRequestPending, signalTeamManageTitle,
signalTeamManageOpen, signalTeamManageClose, signalTeamMemberListHint,
signalTeamAddMember, signalTeamRemoveMember, signalTeamPendingRequests,
signalTeamApproveRequester, mentionInboxTitle) in
packages/i18n/src/messages/fr.json are still in English; replace each value with
the correct French translations consistent with other locales and the UI copy,
and verify all these keys exactly match the corresponding keys in other locale
files so no keys are missing or misnamed. Ensure translations preserve intent
(e.g., “Request to be included” → appropriate French phrasing) and re-run any
i18n validation scripts to confirm consistency across locales.

In `@packages/i18n/src/messages/pt.json`:
- Around line 1990-2002: The Portuguese locale file contains several
untranslated Signal Team strings; update the pt.json entries for
composerLockedPlaceholder, signalTeamInteractionRestricted,
signalTeamBannerReadOnly, signalTeamRequestAccess, signalTeamRequestPending,
signalTeamManageTitle, signalTeamManageOpen, signalTeamManageClose,
signalTeamMemberListHint, signalTeamAddMember, signalTeamRemoveMember,
signalTeamPendingRequests, and signalTeamApproveRequester by providing accurate
Portuguese translations for each key while keeping the exact message keys and
surrounding punctuation unchanged so the locale remains consistent with other
language files.

In `@packages/notifications/src/actions/notify-chat-mention.ts`:
- Around line 23-25: The current guard in notify-chat-mention.ts only checks
that authToken is non-empty (authToken) but does not actually validate it;
update the authentication step in the notifyChatMention function (or the
top-level export that processes mentions) to verify the token against the
trusted auth service or server-side context (call your existing verifyAuthToken
/ validateToken function or perform a remote auth check) and reject if
verification fails; replace the simple presence check with a real verification
call, log/throw a descriptive error when verification returns false, and ensure
downstream code only runs when the token is confirmed valid.
🪄 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: bc99d53c-2fe3-44dd-bc3c-3056ec5b4d30

📥 Commits

Reviewing files that changed from the base of the PR and between cd7a952 and 13e53a7.

📒 Files selected for processing (12)
  • docs/requirements/Features/chat-mentions-notifications-discord-parity/requirements.md
  • packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx
  • packages/epics/src/common/human-right-panel.tsx
  • packages/epics/src/notifications/components/notification-centre-form.tsx
  • packages/i18n/src/messages/de.json
  • packages/i18n/src/messages/en.json
  • packages/i18n/src/messages/es.json
  • packages/i18n/src/messages/fr.json
  • packages/i18n/src/messages/pt.json
  • packages/notifications/src/actions/notify-chat-mention.test.ts
  • packages/notifications/src/actions/notify-chat-mention.ts
  • packages/notifications/src/actions/notify-chat-mention.utils.ts

Comment thread packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx Outdated
Comment thread packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx Outdated
Comment thread packages/epics/src/common/human-right-panel.tsx Outdated
Comment thread packages/epics/src/common/human-right-panel.tsx Outdated
Comment thread packages/i18n/src/messages/en.json
Comment thread packages/i18n/src/messages/es.json Outdated
Comment thread packages/i18n/src/messages/fr.json Outdated
Comment thread packages/i18n/src/messages/pt.json Outdated
Comment thread packages/notifications/src/actions/notify-chat-mention.ts
Alex Prate added 3 commits May 19, 2026 16:15
Restrict signal-thread mentions to selected team members and route non-team call attempts through request-to-join so team members can approve access in-thread.
Replace the onboarding adventure subtitle copy with the new vision-focused message in all supported locales.
Reset ordered list indices in section 11 user stories and implementation slices to satisfy markdownlint MD029.
Alex Prate and others added 17 commits May 19, 2026 22:37
Restore type/priority badges and time/message metadata to the card body so the image header stays clean and tags remain readable.
Remove heavy dark image overlays while expanding priority tint coverage, slightly increase signal image height, and render loaded signals in a single grid so second-row gaps collapse correctly.
Switch signal card footer actions to accent outline with transparent background so call-to-action emphasis stays clear without heavy full-surface color fill.
Render the same icon language used in the new signal form inside signal-card type and priority badges to improve scanability and visual consistency.
Move type/priority badges beside the time/comment metadata row, slightly increase image height, and soften full-surface priority blending while improving icon contrast for better readability.
Keep signal-card type and priority labels readable by restoring foreground text while preserving semantic icon colors through explicit icon-class support in badges.
Move edit/archive controls out of absolute positioning and into the title row layout to keep consistent alignment and prevent overlap with metadata and badges.
…rship

Improve signal cards with richer creator identity cues and auto-seed new signal threads with all available members so team-scoped interactions are enabled by default.

Co-authored-by: Cursor <cursoragent@cursor.com>
Batch signal-team membership edits while the panel is open and emit a single update message on Done that names added and removed members.

Co-authored-by: Cursor <cursoragent@cursor.com>
Make the board tag picker's All tags action reliably clickable in dialog focus traps and normalize board select field heights to match standard input sizing.

Co-authored-by: Cursor <cursoragent@cursor.com>
Harden signal-team state derivation to ignore spoofable marker messages from untrusted senders and persist owner identity explicitly in team update events.

Co-authored-by: Cursor <cursoragent@cursor.com>
Validate mention notification auth tokens with Privy before processing recipients so downstream notification delivery only runs for trusted callers.

Co-authored-by: Cursor <cursoragent@cursor.com>
Replace signal-team m.room.message payload any-casts with an explicit typed event content shape for safer sendEvent usage.

Co-authored-by: Cursor <cursoragent@cursor.com>
Replace block-in-inline markup in the mention consent row to avoid div-inside-span nesting warnings while preserving layout and behavior.

Co-authored-by: Cursor <cursoragent@cursor.com>
Refine the English onboarding subtitle wording to use natural action-oriented phrasing.

Co-authored-by: Cursor <cursoragent@cursor.com>
Adjust signal-team event typing and apply formatting updates so the epics typecheck and main format check jobs pass on PR CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
Type signal-team payloads as RoomMessageEventContent extensions so custom metadata remains typed without violating sendEvent content constraints.

Co-authored-by: Cursor <cursoragent@cursor.com>
@webguru-hypha

Copy link
Copy Markdown
Contributor Author

@CodeRabbit please review

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read key changed files for review
echo "=== notify-chat-mention.ts ==="
cat packages/notifications/src/actions/notify-chat-mention.ts

echo ""
echo "=== notify-chat-mention.utils.ts ==="
cat packages/notifications/src/actions/notify-chat-mention.utils.ts

echo ""
echo "=== notify-chat-mention.test.ts ==="
cat packages/notifications/src/actions/notify-chat-mention.test.ts

echo ""
echo "=== use-notifications.ts ==="
cat packages/notifications/src/hooks/use-notifications.ts

echo ""
echo "=== tags.ts ==="
cat packages/notifications/src/constants/tags.ts

echo ""
echo "=== validation.ts ==="
cat packages/epics/src/notifications/hooks/validation.ts

echo ""
echo "=== notification-centre-form.tsx (relevant parts) ==="
cat packages/epics/src/notifications/components/notification-centre-form.tsx

echo ""
echo "=== use-send-notifications.ts (notifications package) ==="
cat packages/notifications/src/hooks/use-send-notifications.ts

Length of output: 32543


🏁 Script executed:

#!/bin/bash
echo "=== human-right-panel.tsx (key sections) ==="
# It's a large file (+777/-39), so let's look at the mention/notification related parts
rg -n "notifyChatMention\|mentionNavigationNotice\|onScrollTargetNotFound\|signalTeam\|composerLocked\|mentionCandidates\|sendMessage\|requestSignalTeam" packages/epics/src/common/human-right-panel.tsx | head -100

echo ""
echo "=== matrix-provider.tsx - SendMessageResult ==="
rg -n "SendMessageResult\|sendMessage\|getEventIdFromSendResponse" packages/core/src/matrix/client/providers/matrix-provider.tsx | head -50

echo ""
echo "=== use-send-notifications.ts (core) ==="
cat packages/core/src/notifications/client/hooks/use-send-notifications.ts

echo ""
echo "=== en.json mention/notification keys ==="
python3 -c "
import json
with open('packages/i18n/src/messages/en.json') as f:
    d = json.load(f)
nc = d.get('NotificationCentre', {})
print('NotificationCentre:', json.dumps(nc, indent=2))
hcp = d.get('HumanChatPanel', {})
keys_of_interest = {k: v for k, v in hcp.items() if 'mention' in k.lower() or 'signal' in k.lower() or 'composer' in k.lower()}
print('HumanChatPanel relevant:', json.dumps(keys_of_interest, indent=2))
"

echo ""
echo "=== Check for dead commented-out error block in notification-centre-form.tsx ==="
rg -n "error.*Text\|Text.*error\|commented" packages/epics/src/notifications/components/notification-centre-form.tsx

echo ""
echo "=== Check privyClientSingleton module-level state concern ==="
rg -n "privyClientSingleton\|singleton" packages/notifications/src/actions/notify-chat-mention.ts

echo ""
echo "=== signal-section.tsx key parts ==="
rg -n "localStorage\|createSignalHrefWithBoard\|boardFilter\|hideArchived" packages/epics/src/coherence/components/signal-section.tsx | head -60

echo ""
echo "=== Check for missing i18n key mentionConsent usage ==="
rg -rn "mentionConsent\|mentionOnChatMessage" packages/epics/src packages/i18n/src

Length of output: 6388


🏁 Script executed:

#!/bin/bash
echo "=== Find human-right-panel.tsx ==="
fd "human-right-panel.tsx" --type f

echo ""
echo "=== Find matrix-provider.tsx ==="
fd "matrix-provider.tsx" --type f

echo ""
echo "=== Find signal-section.tsx ==="
fd "signal-section.tsx" --type f

echo ""
echo "=== Find notify-chat-mention.ts ==="
fd "notify-chat-mention.ts" --type f

Length of output: 478


🏁 Script executed:

#!/bin/bash
echo "=== human-right-panel.tsx - notifyChatMention and related logic ==="
grep -n "notifyChatMention\|mentionNavigationNotice\|onScrollTargetNotFound\|signalTeam\|composerLocked\|mentionCandidates\|requestSignalTeam\|sendMessage\|eventId\|deepLink\|spaceSlug" packages/epics/src/common/human-right-panel.tsx | head -120

echo ""
echo "=== matrix-provider.tsx - SendMessageResult and eventId ==="
grep -n "SendMessageResult\|getEventIdFromSendResponse\|eventId\|sendMessage\|sendEvent" packages/core/src/matrix/client/providers/matrix-provider.tsx | head -60

echo ""
echo "=== signal-section.tsx - localStorage and board ==="
grep -n "localStorage\|boardFilter\|hideArchived\|createSignalHref\|fromBoardId\|boardTag" packages/epics/src/coherence/components/signal-section.tsx | head -60

echo ""
echo "=== Check commented-out error display block ==="
grep -n "comment\|{/\*\|error.*Text\|Text.*error" packages/epics/src/notifications/components/notification-centre-form.tsx | head -20

echo ""
echo "=== Check for privyClientSingleton module-level concern ==="
grep -n "privyClientSingleton\|let priv\|getPrivyClient" packages/notifications/src/actions/notify-chat-mention.ts

echo ""
echo "=== Check TAG_MENTION_CONSENT usage in action ==="
grep -n "TAG_MENTION_CONSENT\|requiredTags\|true\b" packages/notifications/src/actions/notify-chat-mention.ts

echo ""
echo "=== Check if mentionConsent i18n key is actually used in the UI (only in channels section) ==="
grep -rn "mentionConsent\|channels\.mentionConsent" packages/epics/src/

echo ""
echo "=== Verify all locale files have mentionOnChatMessage ==="
for locale in en de fr es pt; do
  echo "--- $locale ---"
  python3 -c "
import json, sys
with open('packages/i18n/src/messages/$locale.json') as f:
    d = json.load(f)
nc = d.get('NotificationCentre', {})
subs = nc.get('subscriptions', {})
print('mentionOnChatMessage:', subs.get('mentionOnChatMessage', 'MISSING'))
"
done

echo ""
echo "=== Check human-chat-panel-messages.tsx for onScrollTargetNotFound ==="
grep -n "onScrollTargetNotFound\|missCount\|scrollTargetEventId\|threshold" packages/epics/src/common/human-chat-panel/human-chat-panel-messages.tsx | head -30

echo ""
echo "=== Check human-chat-panel-chat-bar.tsx for composerLocked ==="
grep -n "composerLocked\|composerLockedMessage\|canSend\|pushDrafts\|disabled" packages/epics/src/common/human-chat-panel/human-chat-panel-chat-bar.tsx | head -40

Length of output: 15649


@webguru-hypha Thanks for the thorough autofix pass — the previous comments have been addressed cleanly. Here is the full review of the current state of the PR.


Overall Assessment

The core notification pipeline (notifyChatMentionAction, buildMentionEmailBody, sanitizeMentionIds), the consent data model, the form UI, and the i18n coverage are all well-structured. Auth token verification before DB access is correct, Promise.allSettled surface-level error handling is good, and HTML escaping/URL sanitisation is solid. A few items below worth addressing before merge.


1. 🐛 Actor self-notification when actorSlug is undefined

File: packages/notifications/src/actions/notify-chat-mention.ts

actorSlug is declared optional in NotifyChatMentionInput. When it is undefined, the filter:

.filter((slug): slug is string => Boolean(slug && slug !== actorSlug))

evaluates slug !== undefined → always true, so the actor is never excluded and will receive their own mention notification.

Suggested fix: short-circuit when actorSlug is absent or exclude only when the slug is a non-empty string:

.filter(
  (slug): slug is string =>
    Boolean(slug) && (actorSlug == null || slug !== actorSlug),
)

2. 🧹 Dead i18n key: channels.mentionConsent

Files: all five locale JSON files (en, de, fr, es, pt)

The key NotificationCentre.channels.mentionConsent (e.g. "Mention Notifications Consent") is defined in every locale but is never referenced in the component code. The UI uses subscriptions.mentionOnChatMessage.title / .description instead. The orphan key should either be wired up to a label in the form or removed.


3. 🧹 Commented-out error display block

File: packages/epics/src/notifications/components/notification-centre-form.tsx, Lines 414–415

{/* {error && (
  <Text className="text-error-11 text-sm">{error}</Text>

This dead block should be removed — the error prop is already consumed via {error && <FormMessage>{error}</FormMessage>} near the subscribe button and via the button label switching to "Retry".


4. ⚠️ as never cast on translation key

File: packages/epics/src/coherence/components/signal-section.tsx, Line 567

{t.has('boardFilters' as never)
  ? t('boardFilters' as never)

Casting a translation key to never bypasses the typed key check entirely and will silently return an empty string (or throw in strict mode) if the key is missing. The key boardFilters does not appear in the locale JSON files (only boardFilterBy, boardFilterCategory, etc. exist). This block is likely unreachable dead code — it should be removed or replaced with a real typed key.


5. ℹ️ Module-level Privy singleton in a Server Action

File: packages/notifications/src/actions/notify-chat-mention.ts, Lines 14–27

let privyClientSingleton: PrivyClient | null = null;

Module-level mutable state in a Next.js Server Action is safe within a single worker process but will be re-initialised on each cold start in serverless deployments (Vercel, Lambda, etc.), so the "singleton" benefit only applies within a warm invocation. This is not a bug — the code path still works correctly, instantiating a fresh client on each cold start — but it is worth documenting the intent, or using a proper lazy-init utility (once) to make the intent explicit.


✅ Items verified as correct

Area Status
sendResult.eventId gating before notifyChatMention dispatch
Deep link built from canonical spaceSlug
escapeHtml + getSafeMentionHref in email body
Promise.allSettled + partial failure surfacing
TAG_MENTION_CONSENT: 'true' required-tags contract
mentionNotificationsConsent init/reset/save round-trip
All 5 locales have mentionOnChatMessage.title / .description
onScrollTargetNotFound per-eventId guard + miss-count threshold
composerLocked gating for canSend, pushDrafts, drag-drop
Unit tests for sanitizeMentionIds and buildMentionEmailBody

🐇

Alex Prate and others added 10 commits May 20, 2026 02:58
Apply CodeRabbit feedback to provide screen-reader context for signal card message counts while keeping visual output unchanged.

Co-authored-by: Cursor <cursoragent@cursor.com>
Address CodeRabbit i18n feedback by replacing hardcoded resize handle aria/title copy with translation keys.

Co-authored-by: Cursor <cursoragent@cursor.com>
Apply CodeRabbit feedback so mention candidate recomputation is triggered for both thread and parent space room membership updates.

Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve CodeRabbit feedback by translating leftover English strings in OnboardingAdventure.aiHero for Portuguese users.

Co-authored-by: Cursor <cursoragent@cursor.com>
Apply CodeRabbit feedback by using a null SWR key until endpoint and jwt exist, preventing unnecessary refresh loops.

Co-authored-by: Cursor <cursoragent@cursor.com>
Apply CodeRabbit feedback by rendering the onboarding hero title from next-intl and removing hardcoded English heading fragments.

Co-authored-by: Cursor <cursoragent@cursor.com>
Apply CodeRabbit accessibility feedback by running the standard axe-core check in the space-context ecosystem logo scenario.

Co-authored-by: Cursor <cursoragent@cursor.com>
Repair the onboarding adventure component after misapplied branch transfer changes so web typecheck passes again.
Bring back the missing signal card metadata row so type, priority, and creator are consistently visible after recent layout refactors.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant