refactor: set tshow to true for the first thread message in federated rooms

[ricardogarim]

As per FDR-126, added a check on the threadRootMessage.tcount to identify the first message of a thread. If tcount is missing, the message is treated as the first in the thread and tshow is set to true, ensuring it is also sent to the main room. Otherwise, will set tshow to false.

This mirrors the Rocket.Chat behavior, where federated rooms don’t provide a checkbox to choose whether to send a thread message to the main room, but the first message of a thread is always posted there automatically.

Summary by CodeRabbit

• Bug Fixes

  • Skips processing of threaded replies from federated sources when the thread root is missing, preventing orphaned replies.
  • Ensures thread visibility is accurate by removing an implicit default.

• Improvements

  • Carries thread metadata (including visibility) explicitly with incoming federated messages.
  • Adds a warning when a thread root cannot be located to aid diagnosis.

[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

[changeset-bot]

⚠️ No Changeset found

Latest commit: 7be3815

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Replaces bare tmid with a structured thread object ({ tmid, tshow }) in federation message handling and the message service API; handlers now compute and pass explicit thread metadata and return early when the thread root is missing, and persistence calls accept thread instead of top-level tmid.

Changes

Cohort / File(s)	Summary
Message service API adjustment apps/meteor/server/services/messages/service.ts	saveMessageFromFederation signature changed from tmid?: string to thread?: { tmid: string; tshow: boolean }. Removed previous threadParams construction and implicit tshow: true; payload now spreads thread directly into executeSendMessage calls.
Federation Matrix thread handling ee/packages/federation-matrix/src/events/message.ts	Replaced local tmid with optional thread object. For thread messages, fetch thread root; if missing, log a warning and return early. If found, compute tshow = !threadRoot.tcount and set thread = { tmid, tshow }; pass thread in persistence for new and quoted messages and remove old tmid-only flows and associated logs.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Matrix as Matrix Event Handler
  participant DB as Message Store / DB
  participant MsgSvc as MessageService

  Note over Matrix: Incoming Matrix event

  alt Thread message
    Matrix->>DB: fetch thread root by eventId
    alt Root found
      Note right of Matrix #D6EAF8: compute tshow = !threadRoot.tcount
      Matrix->>MsgSvc: saveMessageFromFederation(..., thread={tmid, tshow})
      MsgSvc->>DB: executeSendMessage(..., thread={tmid, tshow})
      DB-->>MsgSvc: persisted
      MsgSvc-->>Matrix: OK
    else Root not found
      Note over Matrix #FDEBD0: log warning and return early
    end
  else Non-thread message
    Matrix->>MsgSvc: saveMessageFromFederation(..., thread=undefined)
    MsgSvc->>DB: executeSendMessage(..., no thread)
    DB-->>MsgSvc: persisted
    MsgSvc-->>Matrix: OK
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• refactor: set tshow to true for the first thread message in federated rooms #37008 — Makes the same change to saveMessageFromFederation and federation event handling to pass { tmid, tshow } instead of a top-level tmid.

Poem

A rabbit hops with careful thread,
I carry { tmid, tshow } ahead.
If root's not found, I pause and say—
"No message lost, I'll wait this day."
Tiny hops, big threads — carrot-fed 🥕

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes Check	⚠️ Warning	The change to saveMessageFromFederation's signature in apps/meteor/server/services/messages/service.ts from tmid?: string to thread?: { tmid: string; tshow: boolean } is an API-level and behavioral change (removing the implicit tshow:true when only tmid was provided) that may affect callers outside the federation path and therefore represents an out-of-scope or at least cross-cutting change relative to the linked issue's narrowly stated objectives. Other edits (thread handling, tshow computation, early-return on missing root) are in-scope for FDR-126, but the public signature change introduces risk of regressions and requires broader coordination.	Make the change backward-compatible by accepting both the legacy tmid?: string and the new thread object or add a thin adapter that maps tmid to { tmid, tshow } for existing callers, perform a repo-wide update of callers if intentional, add tests covering both shapes, and document the API change before merging.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "refactor: set tshow to true for the first thread message in federated rooms" concisely and accurately summarizes the primary change in the changeset: ensuring the first thread message is marked tshow=true in federated rooms; it is specific to the behavior being changed and scoped to federated rooms, and the "refactor" prefix is appropriate.
Linked Issues Check	✅ Passed	The implementation aligns with FDR-126 by checking threadRootMessage.tcount and setting thread.tshow = !threadRootMessage?.tcount so the first thread message is sent to the main room while subsequent messages are not, and it propagates the thread object into persistence calls; a guard was also added to avoid processing when the thread root is missing, which prevents incorrect defaults and matches the issue objective. These code-level changes address the linked issue's core requirements to detect the first thread message and control visibility accordingly FDR-126.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7c6b346 and 7be3815.

📒 Files selected for processing (2)

• apps/meteor/server/services/messages/service.ts (1 hunks)
• ee/packages/federation-matrix/src/events/message.ts (3 hunks)

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.09%. Comparing base (c70e153) to head (adc26b7).

Additional details and impacted files

@@               Coverage Diff                @@
##           feat/federation   #37008   +/-   ##
================================================
  Coverage            69.09%   69.09%           
================================================
  Files                 2959     2959           
  Lines               102190   102190           
  Branches             18259    18261    +2     
================================================
  Hits                 70605    70605           
  Misses               29716    29716           
  Partials              1869     1869           

Flag	Coverage Δ
unit	71.23% <ø> (+<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.

[ggazzo]

I was wondering if changing this true to false, would do the whole magic

[ricardogarim]

to comply with existing RC behavior we also need to check if is first thread message. if so, set tshow: true, otherwise, false