fix(matrix): emit spec-compliant mentions#59323
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2544b757f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Emits Matrix-spec-compliant mention metadata (m.mentions) and HTML mention anchors (matrix.to) across outbound Matrix send/edit flows so Matrix clients can reliably notify/highlight mentions.
Changes:
- Add mention extraction + HTML rendering that produces
m.mentionsandmatrix.toanchors from Markdown. - Route all outbound content types (text, media captions, polls’ fallback text, edits, action-driven edits) through the shared formatting/mention pipeline.
- Add/expand tests covering mention extraction, ambiguity handling, edit mention diffs, and poll mention metadata.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/matrix/src/matrix/send/media.ts | Removes direct formatting call so media captions are enriched via the shared formatting path. |
| extensions/matrix/src/matrix/send/formatting.ts | Introduces async formatting enrichment that attaches m.mentions and formatted_body consistently; adds mention diff helpers for edits. |
| extensions/matrix/src/matrix/send.ts | Applies enrichment to send/edit/poll flows; computes mention deltas for edit events. |
| extensions/matrix/src/matrix/send.test.ts | Adds integration-style tests asserting m.mentions + matrix.to anchors across send/edit/poll/media caption scenarios. |
| extensions/matrix/src/matrix/format.ts | Implements Markdown-based mention detection, bare-localpart resolution via joined members, and HTML token mutation for mention links. |
| extensions/matrix/src/matrix/format.test.ts | Adds unit tests for mention rendering/metadata, ambiguity behavior, and code-span exclusion. |
| extensions/matrix/src/matrix/actions/messages.ts | Routes action-driven edits through the shared edit helper to keep mention behavior consistent. |
| extensions/matrix/src/matrix/actions/messages.test.ts | Adds coverage ensuring action edits preserve mention behavior via the shared helper. |
| CHANGELOG.md | Documents the Matrix mention compliance fix. |
Greptile SummaryThis PR adds spec-compliant The implementation is well-structured, correctly handles qualified MXIDs, bare localpart resolution, ambiguity fallback, Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/matrix/src/matrix/format.ts
Line: 291
Comment:
**`getUserId()` called unconditionally on every send**
`getUserId()` is awaited on every call to `resolveMarkdownMentionState`, even for messages that contain no mentions at all. While the underlying Matrix JS SDK call is synchronous/cached (just reads stored credentials), it still adds an unnecessary async microtask for every outbound message that has no `@` references.
Consider guarding this call similarly to how `getJoinedRoomMembers` is already guarded — only fetch `selfUserId` when at least one user-mention candidate exists in the parsed tokens:
```ts
const hasMentionCandidates = tokens.some((t) =>
t.children?.some((c) => c.type === "text" && collectMentionCandidates(c.content).length > 0)
);
const selfUserId = hasMentionCandidates
? await params.client.getUserId().catch(() => null)
: null;
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/matrix/src/matrix/send.ts
Line: 449-453
Comment:
**Edit of pre-`m.mentions` message over-notifies existing mentions**
When a user edits a message that was sent before this PR (i.e., the original event has no `m.mentions` field), `extractMatrixMentions(previousContent)` returns `{}`. `diffMatrixMentions` then treats every mention in the new content as "new", so all mentioned users receive a notification again — even if they were already mentioned in the original text.
This is an inherent migration concern: there is no way to reconstruct the prior mention set from old events. It's worth documenting this behaviour (e.g. with a code comment) so future maintainers understand why seemingly-duplicate notifications can occur immediately after the rollout.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(matrix): emit spec-compliant mention..." | Re-trigger Greptile |
88b4ca7 to
d2715f3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2715f3adf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1e028ea to
63d39eb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 581b4ae34d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93d6d2d14d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85d863f90d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 357805af07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33b0de8d48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8c00219 to
24d4ebf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24d4ebfd8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
f5c4c3e to
9fee0dc
Compare
9fee0dc to
4b641e3
Compare
|
Merged via squash.
Thanks @gumadeiras! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b641e35a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Merged via squash. Prepared head SHA: 4b641e3 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 4b641e3 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 4b641e3 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 4b641e3 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 4b641e3 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Merged via squash. Prepared head SHA: 4b641e3 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras
Summary
m.mentionsmetadata andmatrix.toanchors for outbound text, media captions, polls, and editsChange Type
Scope
Linked Issue / PR
Closes #56950
Root Cause / Regression History
Outbound Matrix sends never populated
content["m.mentions"]and rendered mentions as plain text HTML, so Element/spec-compliant clients had no structured mention metadata to notify on. Action-driven edits also bypassed the shared Matrix edit helper, and poll fallback text never ran through mention extraction.Regression Test Plan
extensions/matrix/src/matrix/format.test.tsextensions/matrix/src/matrix/send.test.tsextensions/matrix/src/matrix/actions/messages.test.tsUser-visible / Behavior Changes
Matrix mentions now notify reliably across text sends, media captions, edits, poll fallback text, and action-driven edits when the content resolves to
@room, a full MXID, or a unique bare localpart in the room.Security Impact
None.
Repro + Verification
@alice:example.orgor@roomfrom OpenClaw Matrix paths; payloads lackedm.mentions, so clients like Element would not treat them as spec-compliant mentions.m.mentionsandmatrix.toanchors; edit paths also send correct mention deltas.pnpm test -- extensions/matrix/src/matrix/format.test.tspnpm test -- extensions/matrix/src/matrix/send.test.tspnpm test -- extensions/matrix/src/matrix/actions/messages.test.tspnpm buildpnpm checkHuman Verification
Not run.
Compatibility / Migration
No migration needed.
Risks and Mitigations
Risk is limited to Matrix-plugin outbound formatting. Coverage now spans qualified mentions, unique bare-localpart resolution, ambiguity fallback,
@room, poll fallback text, and action/edit routing.