fix: desktop notification quick replies to thread messages posted to the main room

[jeanfbrito]

Proposed changes (including videos or screenshots)

Replying to a desktop notification for a thread message posts the reply to the main room instead of the thread.

The server-side desktop notification payload already carries the thread id (tmid), but the client's useNotification reply handler dropped it when calling sendMessage. This passes it through, using the same notification.payload.tmid conditional the hook's onclick handler already uses for thread navigation. No server change needed.

Also adds the first spec coverage for useNotification — the regression slipped through the sendMessage REST migration (#40675) unnoticed because the hook had no tests:

• reply to a thread notification includes tmid in the sent message
• reply to a room notification sends no tmid key

Issue(s)

Internal support escalation SUP-1097 (Track B). Related Desktop-side fix: RocketChat/Rocket.Chat.Electron#3464

Steps to test or reproduce

1. Enable desktop notifications; have another user reply to you inside a thread while the room is not focused.
2. Use the notification's inline reply field.
3. Before: the reply appears in the main room. After: the reply lands in the thread.

Targeted verification: yarn jest client/hooks/notification/useNotification.spec.ts (2/2 green), ESLint clean on changed files.

Further comments

One-line behavioral change; sendMessage already accepts tmid (AtLeast<IMessage, '_id' | 'rid' | 'msg'>), so no typing changes. Changeset included (@rocket.chat/meteor patch).

Summary by CodeRabbit

• Bug Fixes

  • Fixed desktop notification quick replies so responses to threaded messages remain in the correct thread instead of posting to the main room.
  • Preserved thread context when replying directly from notifications.

• Tests

  • Added coverage for notification permissions, messaging, reply payloads, and thread-specific behavior.

[dionisio-bot]

Looks like this PR is not ready to merge, because of the following issues:

• This PR is missing the 'stat: QA assured' label
• This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5a2ac55

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@rocket.chat/meteor	Patch
@rocket.chat/core-typings	Patch
@rocket.chat/rest-typings	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3651d99c-c1ca-439b-aead-2d9755000d18

📥 Commits

Reviewing files that changed from the base of the PR and between 03546e6 and 5a2ac55.

📒 Files selected for processing (3)

• .changeset/quiet-thread-replies.md
• apps/meteor/client/hooks/notification/useNotification.spec.ts
• apps/meteor/client/hooks/notification/useNotification.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• apps/meteor/client/hooks/notification/useNotification.spec.ts
• .changeset/quiet-thread-replies.md
• apps/meteor/client/hooks/notification/useNotification.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📜 Recent review details

⏰ Context from checks skipped due to timeout. (4)

• GitHub Check: 📦 Build Packages
• GitHub Check: CodeQL-Build
• GitHub Check: Hacktron Security Check
• GitHub Check: CodeQL-Build

---

Walkthrough

Desktop notification replies now include tmid for threaded messages. Tests verify thread and room reply payloads. A patch changeset documents the fix.

Changes

Notification reply context

Layer / File(s)	Summary
Reply payload and validation apps/meteor/client/hooks/notification/useNotification.ts, apps/meteor/client/hooks/notification/useNotification.spec.ts, .changeset/quiet-thread-replies.md	The notification reply payload conditionally includes tmid. Tests verify that thread replies include tmid and room replies omit it. The changeset documents the patch fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 5a2ac

This change preserves the thread identifier for desktop notification replies so responses land in the correct thread; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested labels: type: bug

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the fix for desktop notification replies to thread messages.
Linked Issues check	✅ Passed	The PR implements the linked issue's explicitly assigned main-repository fix by preserving tmid for thread replies and adding tests.
Out of Scope Changes check	✅ Passed	The code, tests, and changeset directly support the thread-reply fix and contain no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)

• SUP-1097: Request failed with status code 401

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cubic-dev-ai]

No issues found across 3 files

_{Re-trigger cubic}

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[jeanfbrito]

Rebased onto current develop — the reply handler was migrated to sdk.rest.post('/v1/chat.sendMessage', ...) upstream (#40675), so the fix now adds tmid inside the REST message body and the spec mocks the REST client. Confirmed chatSendMessageSchema explicitly allows message.tmid (compile-time and ajv). Spec 2/2 green, eslint clean.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.23%. Comparing base (03546e6) to head (5a2ac55).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #41875      +/-   ##
===========================================
+ Coverage    69.20%   69.23%   +0.03%     
===========================================
  Files         4237     4237              
  Lines       167697   167785      +88     
  Branches     29888    29896       +8     
===========================================
+ Hits        116048   116163     +115     
+ Misses       46482    46456      -26     
+ Partials      5167     5166       -1     

Flag	Coverage Δ
e2e	58.93% <0.00%> (-0.02%)	⬇️
e2e-api	46.14% <ø> (+0.25%)	⬆️
unit	71.12% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jeanfbrito]

Unrelated to this PR's change, but found while validating the Desktop side of SUP-1097 and worth a look by someone who knows this hook's history:

useNotification.ts unconditionally closes every desktop notification a fixed time after showing it:

const notificationDuration = !requireInteraction ? (notification.duration ?? 0) - 0 || 10 : -1;
if (notificationDuration > 0) {
  setTimeout(() => n.close(), notificationDuration * 1000);
}

The server never sends duration for desktop notifications (sendNotificationsOnMessage/notifyDesktopUser omit it), so this is always exactly 10 seconds.

On Windows that timer cannot do what it looks like it does: once the toast banner has slid away, the notification lives on as an Action Center card that the OS keeps repliable indefinitely, and close() does not remove it. So the call has no visible effect for the user while telling the desktop app the notification is over — which is how quick replies typed after 10 seconds ended up being dropped (fixed on the Desktop side in RocketChat/Rocket.Chat.Electron#3464, which now keeps its reply routing alive regardless).

Two things worth deciding by someone with the context:

1. Whether this auto-close is still wanted at all — browsers already apply their own notification timeouts, and the platform (at least Windows) does not honour the intent.
2. If it is wanted, whether the 10s fallback should be much longer or platform-aware, since it is effectively a hardcoded constant rather than the server-driven value the code suggests.

Happy to open a separate issue if that is preferable to a comment here.