feat: ✨ 100+ Matrix -> LINE reactions#172
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🧰 Additional context used📓 Path-based instructions (2)**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/!(ltsm)/**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces stub handlers with full Matrix→LINE paid reaction bridging: adds LINE reaction types and RPCs, emoji→sticon mapping and parsing, request-sequence generation/tracking, PreHandle/Handle/Remove implementations, error mapping, sync dedup, and capability reporting. ChangesLINE Reaction Support
Sequence DiagramsequenceDiagram
participant MatrixUser
participant Connector
participant LineClient
participant TalkService
participant Database
MatrixUser->>Connector: PreHandleMatrixReaction(event)
Connector->>Connector: normalizeMatrixReactionKey & map to sticon URL
MatrixUser->>Connector: HandleMatrixReaction(confirm)
Connector->>LineClient: React(reqSeq, messageID, reactionType)
LineClient->>TalkService: react(ReactRequest)
TalkService-->>LineClient: {code,message,data}
LineClient-->>Connector: success / mapped error
Connector->>Database: create Reaction record (on success)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![indent[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
| ref, ok := linePaidReactionForMatrixEmoji(key) | ||
| if !ok { | ||
| return bridgev2.MatrixReactionPreResponse{}, unsupportedMatrixReactionError(key) | ||
| } |
There was a problem hiding this comment.
Predefined reactions are silently routed through the paid path. PreHandleMatrixReaction / HandleMatrixReaction always call linePaidReactionForMatrixEmoji and build a PaidReactionType request, but pkg/line/reaction.go:81-88 declares the six LINE predefined reactions (👍 LIKE=2, ❤ LOVE=3, 😆 LAUGH=4, 😮 SURPRISE=5, 😢 SAD=6, 😡 ANGRY=7) which are also present in lineEmojiReactionURLs. The ReactionType.PredefinedReactionType int field exists in the new request struct but is never set anywhere in the PR.
This matters because paid sticon reactions require the account to have access to the 670e0cce840a8236ddd4ee4c / 5ac1bfd5040ab15980c9b435 packs. If LINE returns the invalid paidreactiontype in reactiontype error you already handle, the bridge falls back to unsupportedMatrixReactionError(key) and the Matrix user sees their thumbs-up/heart fail — even though sending those six as predefinedReactionType would always succeed.
Suggested fix: try predefined first, fall back to paid. Something like:
if n, ok := linePredefinedReactionForMatrixEmoji(key); ok {
return line.ReactionType{PredefinedReactionType: n}, networkid.EmojiID("predef:"+strconv.Itoa(n)), true
}
ref, ok := linePaidReactionForMatrixEmoji(key)
...built off an inverse of PredefinedReactionEmoji (running normalizeMatrixReactionKey over its keys so \u2764\uFE0F resolves).
| func (lc *LineClient) handleOperation(ctx context.Context, op line.Operation) { | ||
| opType := OperationType(op.Type) | ||
|
|
||
| if opType == OpPredefinedReaction || opType == OpReaction { |
There was a problem hiding this comment.
Latent: OpReaction (140) shouldn't be part of the reqSeq dedup. The existing handler at lines 996-1000 already documents that "Type 140 is the 'other' event — param3 is the observer, not the actor." Our self-sent reactions are echoed as OpPredefinedReaction (139); the OpReaction branch only fires for other users' reactions, whose op.ReqSeq is generated by their client.
Since nextReqSeq() is time.Now().UnixMilli() % 1_000_000_000, the value space is small and the comparison is purely numeric. If another participant reacts while we have a recently-tracked but not-yet-echoed reqSeq (e.g. an in-flight client.React/client.CancelReaction, or a client.SendMessage that shares the same map) whose value happens to equal theirs, consumeSentReqSeq(op.ReqSeq) will return true and silently swallow their reaction.
Suggested fix:
if opType == OpPredefinedReaction {
if lc.consumeSentReqSeq(op.ReqSeq) {
return
}
}| if opType == OpPredefinedReaction || opType == OpReaction { | |
| if opType == OpPredefinedReaction { | |
| if lc.consumeSentReqSeq(op.ReqSeq) { | |
| return | |
| } | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@pkg/connector/reaction.go`:
- Around line 196-197: The current parsing assigns version = 0 when strconv.Atoi
fails; change it so the default version (1) is preserved unless parsing
succeeds: after getting rawVersion from parsed.Query().Get("v"), call
strconv.Atoi(rawVersion) and only set the variable version if err == nil (e.g.,
v, err := strconv.Atoi(rawVersion); if err == nil { version = v }), leaving
version unchanged on parse errors or empty input.
- Around line 273-277: The current nextReqSeq() creates collisions by using only
UnixMilli; add a lastReqSeq int field to LineClient and make nextReqSeq generate
a unique seq by taking (time.Now().UnixMilli() % 1_000_000_000) but if it equals
lastReqSeq then increment (wrapping mod 1_000_000_000) until unique, update
lastReqSeq with the chosen value and call trackReqSeq(reqSeq); protect
reads/writes to lastReqSeq with the same mutex used for LineClient state (or add
a mutex) so nextReqSeq() is concurrency-safe and preserves consumeSentReqSeq
dedup behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db8a9c92-627d-4e99-ac05-1d57205e0c05
📒 Files selected for processing (7)
pkg/connector/client.gopkg/connector/reaction.gopkg/connector/sync.gopkg/connector/userinfo.gopkg/line/errors.gopkg/line/methods.gopkg/line/reaction.go
💤 Files with no reviewable changes (1)
- pkg/connector/client.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-docker
- GitHub Check: Lint with 1.25
- GitHub Check: Lint with 1.25
- GitHub Check: build-docker
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Usego fmtfor code formatting across all Go files
Usegoimportswith-local "github.com/highesttt/matrix-line-messenger"flag to group project-local imports correctly
Usezerologfor logging throughout the codebase
Do not useMsgfin logging; useMsgwith structured fields instead
UseStringerinterface where applicable in Go code
Files:
pkg/connector/userinfo.gopkg/line/methods.gopkg/connector/sync.gopkg/line/errors.gopkg/line/reaction.gopkg/connector/reaction.go
**/!(ltsm)/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/!(ltsm)/**/*.go: Runstaticcheckon all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Rungo veton all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Files:
pkg/connector/userinfo.gopkg/line/methods.gopkg/connector/sync.gopkg/line/errors.gopkg/line/reaction.gopkg/connector/reaction.go
🧠 Learnings (1)
📚 Learning: 2026-05-29T10:13:37.093Z
Learnt from: CR
Repo: beeper/matrix-line-messenger PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-29T10:13:37.093Z
Learning: Applies to pkg/connector/connector.go : Implement `bridgev2.NetworkConnector` and `bridgev2.NetworkAPI` interfaces in the connector package for bridge logic
Applied to files:
pkg/connector/reaction.go
🔇 Additional comments (5)
pkg/line/reaction.go (1)
60-78: LGTM!pkg/line/methods.go (1)
392-435: LGTM!pkg/line/errors.go (1)
107-117: LGTM!pkg/connector/sync.go (1)
765-769: LGTM!pkg/connector/userinfo.go (1)
48-48: LGTM!
| if rawVersion := parsed.Query().Get("v"); rawVersion != "" { | ||
| version, _ = strconv.Atoi(rawVersion) |
There was a problem hiding this comment.
Keep default version=1 when query parsing fails.
Line 197 overwrites version with 0 on invalid v query input. Preserve the default unless parsing succeeds.
💡 Proposed fix
version := 1
if rawVersion := parsed.Query().Get("v"); rawVersion != "" {
- version, _ = strconv.Atoi(rawVersion)
+ if parsedVersion, err := strconv.Atoi(rawVersion); err == nil && parsedVersion > 0 {
+ version = parsedVersion
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if rawVersion := parsed.Query().Get("v"); rawVersion != "" { | |
| version, _ = strconv.Atoi(rawVersion) | |
| if rawVersion := parsed.Query().Get("v"); rawVersion != "" { | |
| if parsedVersion, err := strconv.Atoi(rawVersion); err == nil && parsedVersion > 0 { | |
| version = parsedVersion | |
| } | |
| } |
🤖 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 `@pkg/connector/reaction.go` around lines 196 - 197, The current parsing
assigns version = 0 when strconv.Atoi fails; change it so the default version
(1) is preserved unless parsing succeeds: after getting rawVersion from
parsed.Query().Get("v"), call strconv.Atoi(rawVersion) and only set the variable
version if err == nil (e.g., v, err := strconv.Atoi(rawVersion); if err == nil {
version = v }), leaving version unchanged on parse errors or empty input.
| break | ||
| } | ||
| version := 1 | ||
| if rawVersion := parsed.Query().Get("v"); rawVersion != "" { |
There was a problem hiding this comment.
Default version is silently overwritten with 0 on parse failure.
version, _ = strconv.Atoi(rawVersion) discards the error, so a non-empty but non-numeric ?v= query (e.g. ?v=latest) drops version from the intended default of 1 to 0. The map entries in lineEmojiReactionURLs are all hard-coded without a v query today, so this isn't reachable in current code — but the existing branch is dead-defensive logic for URLs that might be added later, and it currently has the opposite effect of the comment-implied intent.
| if rawVersion := parsed.Query().Get("v"); rawVersion != "" { | |
| version := 1 | |
| if rawVersion := parsed.Query().Get("v"); rawVersion != "" { | |
| if v, err := strconv.Atoi(rawVersion); err == nil { | |
| version = v | |
| } | |
| } |
…oji-react-to-line-reactions
|
@highesttt amazing! thanks ❤️ |
2 participants
No description provided.