[Bug fix] Matrix: include sender name in group message envelopes (#27355)#27401
Closed
koushikxd wants to merge 2 commits into
Closed
[Bug fix] Matrix: include sender name in group message envelopes (#27355)#27401koushikxd wants to merge 2 commits into
koushikxd wants to merge 2 commits into
Conversation
Contributor
Greptile SummaryFixes Matrix group message sender disambiguation by switching from Changes:
Issue found:
Confidence Score: 2/5
Last reviewed commit: 1a2742e |
| channel: "Matrix", | ||
| from: envelopeFrom, | ||
| timestamp: eventTs ?? undefined, | ||
| previousTimestamp, | ||
| envelope: envelopeOptions, | ||
| body: textWithId, | ||
| chatType: threadRootId ? "thread" : isDirectMessage ? "direct" : "channel", |
Contributor
There was a problem hiding this comment.
"thread" is not a recognized chat type and will be normalized to undefined by normalizeChatType(), causing thread messages to be treated as direct messages (no sender prefix). Use "channel" instead, matching Discord and Slack's pattern
Suggested change
| chatType: threadRootId ? "thread" : isDirectMessage ? "direct" : "channel", | |
| chatType: isDirectMessage ? "direct" : "channel", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/matrix/src/matrix/monitor/handler.ts
Line: 508
Comment:
`"thread"` is not a recognized chat type and will be normalized to `undefined` by `normalizeChatType()`, causing thread messages to be treated as direct messages (no sender prefix). Use `"channel"` instead, matching Discord and Slack's pattern
```suggestion
chatType: isDirectMessage ? "direct" : "channel",
```
How can I resolve this? If you propose a fix, please make it concise.1a2742e to
a0aa104
Compare
Contributor
|
Landed to Closing this PR as superseded-by-landed. Thanks @koushikxd for the report + patch direction. |
Author
|
@steipete Awesome, thanks for picking it up! |
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
2 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.
Fixes the bug where the bot resolves "me/I" to the wrong user in Matrix group chats (#27355).
What changed
In
extensions/matrix/src/matrix/monitor/handler.ts, the message body passed to the session transcript was built usingformatAgentEnvelope, which put only the room name in the header — no sender. So the LLM couldn't tell who sent which message in shared group sessions and fell back to whoever had messaged last.Switched to
formatInboundEnvelopewithchatType+senderparams, which prepends the sender's display name to the body for non-DM messages. This is exactly how Telegram, Discord, and Signal already handle it.Before:
[Matrix MyRoom 08:25 UTC] show me my recent code commits
After:
[Matrix MyRoom 08:25 UTC] Bu: show me my recent code commits
DMs are unaffected (
chatType: "direct"skips the sender prefix). Thread messages also correctly get the sender name.Testing
pnpm build && pnpm check && pnpm testpasses