[codex] Add mautrix iMessage bridge#97
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an end-to-end bridgev2 iMessage connector: Swift C‑ABI runtime (IMessageBridgeKit), Darwin CGO wrapper and typed Go imessage client, connector bootstrap/config/login, capability/chat resolution, state‑sync handling and conversion to Matrix, Matrix→iMessage handlers, commands, and backfill/polling logic. ChangesBridgev2 iMessage connector
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
batuhan
force-pushed
the
batuhan/add-imessage-bridge
branch
from
e0828ff to
ce06d84
Compare
![indent[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
| continue | ||
| } | ||
| for i := len(messages.Items) - 1; i >= 0; i-- { | ||
| message := messages.Items[i] |
There was a problem hiding this comment.
Heavy re-sync on every reconnect: This loop pulls a full message page for every chat and pushes each message into handleMessageStateSync on every Connect. mautrix dedupes via the HandleExistingFunc returning an empty UpsertResult, but the Swift DB query, JSON round-trip, and event-queue pressure still happen each time. Suggest only sending ChatResync here and letting FetchMessages (backfill) drive message catch-up.
| return nil, err | ||
| } | ||
|
|
||
| return &bridgev2.MatrixMessageResponse{ |
There was a problem hiding this comment.
Multi-message sends only record the first ID: firstSentMessage(sent) ignores any remaining entries that Swift returned. When their echoes come back through handleMessageStateSync (upsert), there's no existing database.Message for those IDs, so mautrix will create brand-new Matrix messages for them — the user's own send shows up duplicated in the room. Suggest persisting one DB row (or Parts entry) per returned message ID.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
pkg/connector/capabilities.go (1)
28-30: ⚡ Quick winReturn a defensive copy from
GetCapabilities.Returning the shared
generalCapspointer allows accidental mutation of global capability state by any caller.Suggested fix
func (c *Connector) GetCapabilities() *bridgev2.NetworkGeneralCapabilities { - return generalCaps + caps := *generalCaps + return &caps }🤖 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/capabilities.go` around lines 28 - 30, GetCapabilities currently returns the shared pointer generalCaps allowing callers to mutate global state; change GetCapabilities to return a defensive copy of generalCaps (e.g., create and return a new bridgev2.NetworkGeneralCapabilities instance populated from generalCaps or use proto.Clone(generalCaps) if these are protobuf types) so callers receive an independent object and cannot modify the shared generalCaps.pkg/connector/client.go (2)
86-91: 💤 Low valueEvent polling uses fixed retry delay without backoff.
Line 89 always sleeps 5 seconds after a polling error. If errors persist (e.g., service outage), this fixed interval could generate excessive retry traffic. Consider exponential backoff or a maximum retry count before entering a longer pause or disconnected state.
🤖 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/client.go` around lines 86 - 91, The polling loop currently uses a fixed 5s sleep after c.IM.NextEvents errors which can cause excessive retry traffic; change the error handling in the event polling loop (where c.IM.NextEvents and c.UserLogin.Log.Warn are called) to implement an exponential backoff with a capped delay and a max retry count: track a retry counter that increases on each error, compute sleep = min(baseDelay * 2^retries, maxDelay), reset retries to 0 when events are received successfully, and when retries exceed maxRetries transition to a longer pause or emit a disconnected state (or return an error) so callers can handle it. Ensure backoff values (baseDelay, maxDelay, maxRetries) are configurable constants or fields and that logging includes the current retry count/delay for observability.
116-147: 🏗️ Heavy liftInitial sync only fetches the first page of chats and messages.
Lines 117 and 129 pass
nilpagination, fetching only the first page of chats and the first page of messages per chat. If the user has many chats or messages, most history won't sync on startup. Consider paginating through all chats and using the backfill system for message history instead of syncing all messages upfront.🤖 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/client.go` around lines 116 - 147, The initial syncExistingChats method only calls c.IM.Chats(nil) and c.IM.Messages(thread.ID, nil) which fetch just the first page; change it to iterate chat pages using the pagination token returned by Chats (e.g., loop using page.Next or page.Pagination) and call c.reIDKnownSyntheticPortals / QueueRemoteEvent for each thread as you discover them; for messages do not pull all pages inline—replace the immediate c.IM.Messages pagination with enqueuing backfill requests (or invoking the existing backfill queueing API) for that thread so history is backfilled asynchronously instead of loading all message pages on startup (update references in syncExistingChats and where you currently call c.IM.Messages and c.IM.Chats).pkg/connector/messageconv.go (1)
168-182: 💤 Low valueStickers could use Matrix's native sticker message type.
Line 169-170 maps
attachment.IsStickertoevent.MsgImage, but Matrix supports a dedicatedevent.MsgStickertype that clients can render specially. Consider using the native sticker type for better client compatibility.🤖 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/messageconv.go` around lines 168 - 182, The function msgTypeForAttachment currently maps attachment.IsSticker to event.MsgImage; change that branch to return the dedicated event.MsgSticker so stickers use Matrix's native sticker message type (update the IsSticker handling in msgTypeForAttachment to return event.MsgSticker and ensure the event package defines MsgSticker).pkg/connector/handlematrix.go (1)
160-168: 💤 Low valueConfirm
MaxReactions: 1matches iMessage tapback behavior
iMessage only allows one Tapback per user per message (choosing another Tapback replaces the prior one rather than adding more), soMaxReactions: 1inpkg/connector/handlematrix.gois appropriate; add a brief comment documenting this limitation if useful.🤖 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/handlematrix.go` around lines 160 - 168, The PreHandleMatrixReaction handler currently sets MaxReactions: 1 which is correct for iMessage tapbacks; add a short inline comment in the PreHandleMatrixReaction function (next to the MaxReactions assignment or above the return) stating that iMessage allows only one tapback per user per message and choosing another replaces the previous reaction so we enforce MaxReactions = 1; reference the PreHandleMatrixReaction function and the MaxReactions field when adding this comment.
🤖 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/backfill.go`:
- Around line 88-99: The nextBackfillCursor function does not account for
pagination direction and always returns the oldest cursor or first message ID,
which is incorrect for forward pagination. Modify the function to accept a
parameter indicating the pagination direction, then implement conditional logic
to return page.NewestCursor or the last message ID
(messages[len(messages)-1].ID) when pagination is forward, while keeping the
current logic (page.OldestCursor or first message ID) for backward pagination.
Update the call site around line 54 to pass the appropriate pagination direction
parameter to nextBackfillCursor.
In `@pkg/connector/handleimessage.go`:
- Around line 103-114: The delete branch currently calls c.eventMeta with a stub
imessage.Message causing sender() to default to IsFromMe; instead populate the
Message.ParticipantID with the deleting participant ID when creating the
Reaction event so sender() resolves correctly—i.e., in the "delete" case set
imessage.Message{ThreadID: evt.ObjectIDs.ThreadID, ParticipantID: <use
evt.Actor.ParticipantID or the participant id provided on the event/reaction
payload>} before calling c.eventMeta, mirroring how the upsert path uses
reaction.ParticipantID, and then enqueue the simplevent.Reaction as before.
In `@pkg/connector/handlematrix.go`:
- Around line 170-182: HandleMatrixReaction currently sets Reaction.Timestamp to
time.Now(), losing the original Matrix event time; instead read the event
timestamp from the incoming msg (the bridgev2.MatrixReaction struct passed into
HandleMatrixReaction) — e.g. use the field that carries the event time
(msg.Timestamp or msg.Event.OriginServerTS / msg.Event.Timestamp depending on
the struct) and assign/convert that value into the database.Reaction.Timestamp
(convert from ms/epoch to time.Time if needed) so the stored reaction preserves
the original Matrix event time.
- Around line 31-76: The fallback to createChatFromSyntheticPortal in
HandleMatrixMessage is currently only allowed when quotedMessageID == "" which
prevents retries for synthetic portals and hides original SendText errors;
change the condition so the retry runs when the portal is actually synthetic or
when SendText returns an error that indicates a missing/invalid thread.
Concretely: replace the quotedMessageID == "" check around the SendText error
path with a check like isSyntheticPortal(string(msg.Portal.ID)) (or implement a
helper method on Client to detect synthetic portals from msg.Portal.ID/Name) OR
inspect the returned error for a thread-not-found/invalid-thread sentinel and
call c.createChatFromSyntheticPortal(string(msg.Portal.ID), msg.Content.Body,
msg.Portal.Name) in those cases (still passing quotedMessageID as-is). Ensure
this change affects the SendText error branch in HandleMatrixMessage and keep
other logic (media handling and unsupported types) unchanged.
In `@pkg/connector/login.go`:
- Around line 53-67: Before calling imessageid.MakeUserLoginID with
currentUser.ID, validate that currentUser.ID is non-empty and return a clear
error (or handle appropriately) if it is empty; update the code around creation
of loginID so it checks currentUser.ID, logs or returns an error early, and only
calls imessageid.MakeUserLoginID and l.User.NewLogin when the ID is present
(references: currentUser.ID, loginID, imessageid.MakeUserLoginID,
l.User.NewLogin, database.UserLogin).
In `@pkg/connector/messageconv.go`:
- Around line 78-84: In attachmentBytes, the code calls
c.IM.LoadAttachment(msg.ID) but continues using the original value-type
attachment (so attachment.SrcURL stays empty); after calling
c.IM.LoadAttachment(msg.ID) reload the message/attachment data and use the
refreshed attachment with a populated SrcURL (e.g., fetch the message again via
the IM client or replace attachment by searching msg.Attachments for
attachment.ID after reloading) before proceeding; alternatively, change
LoadAttachment to return the resolved attachment and use that return
value—update Client.attachmentBytes to use the refreshed attachment.SrcURL and
related fields before attempting to download/convert bytes.
In `@pkg/imessage/client.go`:
- Around line 1-8: The package imports
github.com/beeper/platform-imessage/pkg/imessage/lib which currently only has
lib_darwin.go with //go:build darwin && cgo, causing build-constraint errors on
other platforms; either add the same build tag //go:build darwin && cgo to
pkg/imessage/client.go (and any other files importing lib) so those files are
only built on darwin+cgo, or create non-darwin/!cgo stub files in package
imessage/lib (e.g., lib_nondarwin.go) that provide the same exported symbols
used by client.go but return a clear unsupported-platform error (refer to the
import line and any functions/methods in client.go that call into lib, and
mirror their signatures in the stubs).
In `@src/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift`:
- Around line 72-82: startEvents() currently sets eventsStarted via
markEventsStarted() but never resets it if
api.startEventWatchingFromCurrentState() throws, causing future calls to no-op;
wrap the await call in a do/catch (or use defer) so that on any throw you reset
the started flag (either call an existing counterpart like
markEventsStopped()/resetEventsStarted() or add one to set the flag back to
false) before rethrowing the error, keeping api.subscribeToEvents registration
as-is.
---
Nitpick comments:
In `@pkg/connector/capabilities.go`:
- Around line 28-30: GetCapabilities currently returns the shared pointer
generalCaps allowing callers to mutate global state; change GetCapabilities to
return a defensive copy of generalCaps (e.g., create and return a new
bridgev2.NetworkGeneralCapabilities instance populated from generalCaps or use
proto.Clone(generalCaps) if these are protobuf types) so callers receive an
independent object and cannot modify the shared generalCaps.
In `@pkg/connector/client.go`:
- Around line 86-91: The polling loop currently uses a fixed 5s sleep after
c.IM.NextEvents errors which can cause excessive retry traffic; change the error
handling in the event polling loop (where c.IM.NextEvents and
c.UserLogin.Log.Warn are called) to implement an exponential backoff with a
capped delay and a max retry count: track a retry counter that increases on each
error, compute sleep = min(baseDelay * 2^retries, maxDelay), reset retries to 0
when events are received successfully, and when retries exceed maxRetries
transition to a longer pause or emit a disconnected state (or return an error)
so callers can handle it. Ensure backoff values (baseDelay, maxDelay,
maxRetries) are configurable constants or fields and that logging includes the
current retry count/delay for observability.
- Around line 116-147: The initial syncExistingChats method only calls
c.IM.Chats(nil) and c.IM.Messages(thread.ID, nil) which fetch just the first
page; change it to iterate chat pages using the pagination token returned by
Chats (e.g., loop using page.Next or page.Pagination) and call
c.reIDKnownSyntheticPortals / QueueRemoteEvent for each thread as you discover
them; for messages do not pull all pages inline—replace the immediate
c.IM.Messages pagination with enqueuing backfill requests (or invoking the
existing backfill queueing API) for that thread so history is backfilled
asynchronously instead of loading all message pages on startup (update
references in syncExistingChats and where you currently call c.IM.Messages and
c.IM.Chats).
In `@pkg/connector/handlematrix.go`:
- Around line 160-168: The PreHandleMatrixReaction handler currently sets
MaxReactions: 1 which is correct for iMessage tapbacks; add a short inline
comment in the PreHandleMatrixReaction function (next to the MaxReactions
assignment or above the return) stating that iMessage allows only one tapback
per user per message and choosing another replaces the previous reaction so we
enforce MaxReactions = 1; reference the PreHandleMatrixReaction function and the
MaxReactions field when adding this comment.
In `@pkg/connector/messageconv.go`:
- Around line 168-182: The function msgTypeForAttachment currently maps
attachment.IsSticker to event.MsgImage; change that branch to return the
dedicated event.MsgSticker so stickers use Matrix's native sticker message type
(update the IsSticker handling in msgTypeForAttachment to return
event.MsgSticker and ensure the event package defines MsgSticker).
🪄 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: 9ec4195d-6c8a-4f18-b669-d17438dfa06e
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (23)
Package.swiftbridge-readme.mdcmd/mautrix-imessage/main.gogo.modpkg/connector/backfill.gopkg/connector/capabilities.gopkg/connector/chatinfo.gopkg/connector/client.gopkg/connector/config.gopkg/connector/connector.gopkg/connector/example-config.yamlpkg/connector/handleimessage.gopkg/connector/handlematrix.gopkg/connector/login.gopkg/connector/messageconv.gopkg/imessage/client.gopkg/imessage/lib/lib_darwin.gopkg/imessage/types.gopkg/imessageid/dbmeta.gopkg/imessageid/ids.gosrc/IMessage/Sources/IMessage/UI/SettingsView.swiftsrc/IMessage/Sources/IMessage/WindowCoordination/WindowCoordinators/Eclipsing/EclipsingDebuggerView.swiftsrc/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-05-03T17:00:19.662Z
Learnt from: KishanBagaria
Repo: beeper/platform-imessage PR: 69
File: src/IMessage/Sources/IMessage/EventWatcher/EventWatcher+Updates.swift:89-93
Timestamp: 2026-05-03T17:00:19.662Z
Learning: In the beeper/platform-imessage Swift codebase, keep message IDs (`PlatformSDK.MessageID`) as raw GUIDs. When mapping from DB/event rows to `message.id`, set the ID directly from `msgRow.guid` (no GUID→public-ID hashing or transformation). For multi-part messages, append the part index as `_<part.index>` to the GUID-derived ID. During code review, if changes touch message ID creation/mapping, ensure this raw GUID + optional `_<part.index>` suffix behavior is preserved.
Applied to files:
src/IMessage/Sources/IMessage/UI/SettingsView.swiftsrc/IMessage/Sources/IMessage/WindowCoordination/WindowCoordinators/Eclipsing/EclipsingDebuggerView.swiftsrc/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift
📚 Learning: 2026-05-03T17:00:24.877Z
Learnt from: KishanBagaria
Repo: beeper/platform-imessage PR: 69
File: src/IMessage/Sources/IMessage/EventWatcher/EventWatcher+Updates.swift:89-93
Timestamp: 2026-05-03T17:00:24.877Z
Learning: In the `platform-imessage` codebase (Swift), message IDs (`PlatformSDK.MessageID`) are raw GUIDs — they are not hashed or transformed. `message.id` is set directly from `msgRow.guid` in `MessageMapper.swift`, with multi-part messages appending `_<part.index>` as a suffix. There is no GUID→public ID hashing step for messages.
Applied to files:
pkg/imessageid/ids.go
🪛 golangci-lint (2.12.2)
pkg/imessage/client.go
[error] 7-7: could not import github.com/beeper/platform-imessage/pkg/imessage/lib (-: build constraints exclude all Go files in pkg/imessage/lib)
(typecheck)
🪛 SwiftLint (0.63.3)
src/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift
[Warning] 104-104: Magic numbers should be replaced by named constants
(no_magic_numbers)
🔇 Additional comments (45)
pkg/connector/connector.go (1)
24-87: LGTM!pkg/connector/config.go (1)
9-29: LGTM!pkg/connector/example-config.yaml (1)
1-12: LGTM!cmd/mautrix-imessage/main.go (1)
9-26: LGTM!pkg/connector/chatinfo.go (1)
65-94: HandleChats(nil)paging to avoid missing contacts beyond the first page (pkg/connector/chatinfo.go, lines 65-94).
Client.Chats(pagination *Pagination)supports cursor-based paging and returnsPagewithHasMore,OldestCursor, andNewestCursor, butcontactResponsescallsc.IM.Chats(nil)and processes onlypage.Itemswithout checkingHasMore/cursors—so results may be truncated unlessnilpagination is defined to “fetch all”.pkg/imessage/types.go (1)
5-120: LGTM!pkg/imessageid/dbmeta.go (1)
3-24: LGTM!pkg/imessageid/ids.go (1)
10-44: LGTM!Package.swift (1)
38-42: LGTM!Also applies to: 110-118
go.mod (1)
1-39: LGTM!bridge-readme.md (1)
1-21: LGTM!src/IMessage/Sources/IMessage/UI/SettingsView.swift (1)
281-286: LGTM!src/IMessage/Sources/IMessage/WindowCoordination/WindowCoordinators/Eclipsing/EclipsingDebuggerView.swift (1)
207-220: LGTM!src/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift (1)
1-71: LGTM!Also applies to: 84-503
pkg/imessage/lib/lib_darwin.go (1)
1-253: LGTM!pkg/imessage/client.go (1)
10-242: LGTM!pkg/connector/handleimessage.go (6)
19-85: LGTM!
87-102: LGTM!
119-147: LGTM!
149-167: LGTM!
191-242: LGTM!
169-189: ⚡ Quick winDeduplicate participant IDs before generating synthetic portal IDs (fix DM/group classification + portal re-ID mismatches)
syntheticPortalIDsForThreadcollectsthread.Participants.Items[].IDand sorts/joins without deduping, so duplicate participant IDs will:
- change the
joinedstring and the length check (len(participants) == 1vs> 1), potentially switching betweenany;-;/iMessage;-;/SMS;-;andgroup;-;- feed that synthetic value into
reIDKnownSyntheticPortals→Bridge.ReIDPortalas the source portal key, which can prevent correct re-ID if the provisional portal key was built from unique participant IDs (whilechatInfoFromThreadeffectively dedupes viamemberMap.Set).pkg/connector/handleimessage.go:169-189
pkg/connector/messageconv.go (5)
20-49: LGTM!
51-76: LGTM!
107-142: LGTM!
153-166: LGTM!
184-199: LGTM!pkg/connector/handlematrix.go (7)
78-114: LGTM!
116-130: LGTM!
132-154: LGTM!
156-158: LGTM!
184-228: LGTM!
230-242: LGTM!
197-206: ⚡ Quick winAdd/justify a
msg.Portalnil guard inHandleMatrixReadReceipt
HandleMatrixReadReceiptcallsstring(msg.Portal.ID)without checkingmsg.Portal, whileHandleMatrixViewingChatexplicitly returns early onmsg.Portal == nil. Add a nil check (or document a framework guarantee) to avoid a potential nil dereference.pkg/connector/client.go (8)
19-30: LGTM!
31-42: LGTM!
51-71: LGTM!
73-85: LGTM!
92-98: LGTM!
100-114: LGTM!
149-167: LGTM!
43-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEvent goroutines start even when
StartEventsfails.Lines 43-46 log a warning if
StartEventsfails but still proceed to launchsyncExistingChatsandeventLoop(lines 47-48). The event loop will repeatedly callNextEvents, which likely returns errors if event watching never started. This wastes resources and floods logs.Proposed fix
if err := c.IM.StartEvents(); err != nil { c.UserLogin.Log.Warn().Err(err).Msg("Failed to start iMessage event watcher") - return + c.UserLogin.BridgeState.Send(status.BridgeState{ + StateEvent: status.StateTransientDisconnect, + Error: "EVENT_WATCHER_FAILED", + Message: err.Error(), + }) + return } go c.syncExistingChats() go c.eventLoop()> Likely an incorrect or invalid review comment.pkg/connector/backfill.go (3)
15-58: LGTM!
60-76: LGTM!
78-86: LGTM!
| func (c *Client) HandleMatrixMessage(ctx context.Context, msg *bridgev2.MatrixMessage) (*bridgev2.MatrixMessageResponse, error) { | ||
| var ( | ||
| sent []imessage.Message | ||
| err error | ||
| ) | ||
|
|
||
| quotedMessageID := "" | ||
| if msg.ReplyTo != nil { | ||
| quotedMessageID = string(msg.ReplyTo.ID) | ||
| } | ||
|
|
||
| if msg.Content.MsgType.IsText() { | ||
| sent, err = c.IM.SendText(string(msg.Portal.ID), msg.Content.Body, quotedMessageID) | ||
| if err != nil && quotedMessageID == "" { | ||
| sent, err = c.createChatFromSyntheticPortal(string(msg.Portal.ID), msg.Content.Body, msg.Portal.Name) | ||
| } | ||
| } else if msg.Content.MsgType.IsMedia() { | ||
| var tempPath string | ||
| tempPath, err = c.downloadMatrixMedia(ctx, msg) | ||
| if err == nil { | ||
| defer os.Remove(tempPath) | ||
| sent, err = c.IM.SendFile(string(msg.Portal.ID), tempPath, quotedMessageID) | ||
| } | ||
| } else { | ||
| return nil, matrixUnsupported("unsupported iMessage Matrix message type") | ||
| } | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| first, err := firstSentMessage(sent) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return &bridgev2.MatrixMessageResponse{ | ||
| DB: &database.Message{ | ||
| ID: imessageid.MakeMessageID(first.ID), | ||
| SenderID: c.GetUserID(), | ||
| Timestamp: messageTimestamp(*first), | ||
| Metadata: &imessageid.MessageMetadata{ | ||
| ThreadID: string(msg.Portal.ID), | ||
| }, | ||
| }, | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what errors SendText can return and whether synthetic portal detection exists
rg -n -C5 'SendText.*error' --type=go
ast-grep --pattern 'func ($$$) SendText($$$) ($$$, error) { $$$ }'Repository: beeper/platform-imessage
Length of output: 1870
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect SendText implementations and error generation paths
fd -a "lib_*.go" pkg/imessage/lib | head -n 50 >/dev/null
rg -n "func SendText\\(" pkg/imessage/lib -S
rg -n "failed|error|return nil, err|fmt\\.Errorf|errors\\.|var .*Err" pkg/imessage/lib/lib_*.go -S
rg -n "decodeSendResult" pkg/imessage/client.go pkg/imessage -S
sed -n '70,140p' pkg/imessage/client.go
# Find how HandleMatrixMessage triggers synthetic portal chat creation
rg -n "createChatFromSyntheticPortal\\(" -S pkg/connector/handlematrix.go pkg/connector
rg -n "func \\(c \\*Client\\) createChatFromSyntheticPortal" -S pkg
# Identify what the current fallback condition is and any related helpers
sed -n '1,120p' pkg/connector/handlematrix.goRepository: beeper/platform-imessage
Length of output: 8826
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant sections for SendText error handling and decodeSendResult
sed -n '1,220p' pkg/imessage/lib/lib_darwin.go | sed -n '100,190p'
sed -n '200,320p' pkg/imessage/client.go
sed -n '1,140p' pkg/connector/handlematrix.goRepository: beeper/platform-imessage
Length of output: 8686
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the bridge implementation points / any known error strings related to send_text
rg -n "imessage_bridge_send_text" -S .
rg -n "bridge_send_text" -S .
rg -n "quotedMessageID|quotedMessage" -S pkg/imessage pkg/connector .
rg -n "SendText\\(" -S pkg/imessage pkg/connector | head -n 50
# Look for any code that detects "synthetic" and/or retries createChat for synthetic portals
rg -n "synthetic" -S pkg/connector pkg/imessage
# Also inspect createChatFromSyntheticPortal callers/usage
rg -n "createChatFromSyntheticPortal\\(" -S pkg/connectorRepository: beeper/platform-imessage
Length of output: 13377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Swift bridge entrypoint for send_text
sed -n '250,340p' src/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift
# Inspect Swift PlatformAPI sendMessage to see how failures/errors/retries are handled
rg -n "func sendMessage\\(" -S src/IMessage/Sources/IMessage/PlatformAPI.swift
sed -n '250,380p' src/IMessage/Sources/IMessage/PlatformAPI.swift
# Inspect Go synthetic portal identification / re-ID logic
sed -n '120,240p' pkg/connector/handleimessage.go
rg -n "syntheticPortalIDsForThread\\(" -S pkg/connector/handleimessage.go
sed -n '1,120p' pkg/connector/handleimessage.go
# Inspect any use of createChatFromSyntheticPortal besides HandleMatrixMessage
rg -n "createChatFromSyntheticPortal\\(" -S pkg/connectorRepository: beeper/platform-imessage
Length of output: 16863
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate originalThreadID(for:) implementation and any synthetic/provisional handling
rg -n "func originalThreadID\\(for:" -S src/IMessage/Sources/IMessage/PlatformAPI.swift
rg -n "originalThreadID\\(for:" -S src/IMessage/Sources/IMessage/PlatformAPI.swift
sed -n '1,220p' src/IMessage/Sources/IMessage/PlatformAPI.swiftRepository: beeper/platform-imessage
Length of output: 50
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Re-run search for originalThreadID(for:) in PlatformAPI.swift (and related Swift files)
rg -n "originalThreadID\\(for:" -S src/IMessage/Sources/IMessage
rg -n "originalThreadID\\(" -S src/IMessage/Sources/IMessage/PlatformAPI.swift
rg -n "struct ErrorMessage|ErrorMessage\\(" -S src/IMessage/Sources/IMessage/PlatformAPI.swift
rg -n "ErrorMessage\\(" -S src/IMessage/Sources/IMessage | head -n 50
# Show the bottom/top of PlatformAPI.swift around where originalThreadID is likely defined
sed -n '1,220p' src/IMessage/Sources/IMessage/PlatformAPI.swift
sed -n '220,420p' src/IMessage/Sources/IMessage/PlatformAPI.swiftRepository: beeper/platform-imessage
Length of output: 29498
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print PlatformAPI.swift around originalThreadID(for:) and any related synthetic/provisional handling.
sed -n '560,700p' src/IMessage/Sources/IMessage/PlatformAPI.swift
sed -n '700,880p' src/IMessage/Sources/IMessage/PlatformAPI.swiftRepository: beeper/platform-imessage
Length of output: 13323
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "originalThreadID\\(db:" -S src/IMessage/Sources/IMessage/PlatformAPI.swift
# Show around that definition
line=$(rg -n "originalThreadID\\(db:" -S src/IMessage/Sources/IMessage/PlatformAPI.swift | head -n1 | cut -d: -f1)
start=$((line-60))
end=$((line+140))
sed -n "${start},${end}p" src/IMessage/Sources/IMessage/PlatformAPI.swiftRepository: beeper/platform-imessage
Length of output: 9442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/IMessage/Sources/IMessage/PlatformAPI.swift"
loc=$(rg -n "static func originalThreadID\\(db:" -S "$file" | head -n1 | cut -d: -f1 || true)
[ -n "$loc" ] && { s=$((loc-80)); e=$((loc+140)); sed -n "${s},${e}p" "$file"; } || {
# fall back: any "originalThreadID(db:" definition (non-static / different signature)
loc=$(rg -n "originalThreadID\\(db:" -S "$file" | head -n1 | cut -d: -f1)
s=$((loc-80)); e=$((loc+140))
sed -n "${s},${e}p" "$file"
}Repository: beeper/platform-imessage
Length of output: 9623
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/IMessage/Sources/IMessage/PlatformAPI.swift"
# Find exact definition lines
rg -n "originalThreadID\\(db: db, publicThreadID\\)" -S "$file" || true
rg -n "func originalThreadID\\(db:" -S "$file" || true
rg -n "static func originalThreadID\\(db:" -S "$file" || true
rg -n "originalThreadID\\(db:" -S "$file" | head -n 20
# Print around the most likely definition (static/non-static) after we locate it
loc=$(rg -n "originalThreadID\\(db:" -S "$file" | head -n 1 | cut -d: -f1)
if [ -n "${loc:-}" ]; then
s=$((loc-120)); e=$((loc+220))
sed -n "${s},${e}p" "$file"
fiRepository: beeper/platform-imessage
Length of output: 14988
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the actual implementation of Self.originalThreadID(db:publicThreadID)
rg -n "originalThreadID\\(db:\\s*db, publicThreadID" -S src/IMessage/Sources/IMessage || true
rg -n "originalThreadID\\(db:.*publicThreadID" -S src/IMessage/Sources/IMessage || true
rg -n "static func originalThreadID\\(db:" -S src/IMessage/Sources/IMessage || true
rg -n "private static func originalThreadID\\(db:" -S src/IMessage/Sources/IMessage || true
# Also search for "originalThreadID(db:" without assuming exact signature
rg -n "originalThreadID\\(db:" -S src/IMessage/Sources/IMessage/PlatformAPI.swift
# If still only wrapper calls, broaden search to the whole iMessage Swift sources
rg -n "originalThreadID\\(db:" -S src/IMessage/Sources/IMessageRepository: beeper/platform-imessage
Length of output: 2917
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/IMessage/Sources/IMessage/Hashing/PlatformAPI+Hashing.swift"
wc -l "$file"
sed -n '1,200p' "$file"Repository: beeper/platform-imessage
Length of output: 6692
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "sentMessageIDs\\(" -S src/IMessage/Sources/IMessage | head -n 50
rg -n "DatabaseTickWaits\\.sentMessageIDs" -S src/IMessage/Sources/IMessage
rg -n "struct DatabaseTickWaits|class DatabaseTickWaits|enum DatabaseTickWaits" -S src/IMessage/Sources/IMessage
# Print around DatabaseTickWaits definition
file=$(rg -n "DatabaseTickWaits" -S src/IMessage/Sources/IMessage | head -n 1 | cut -d: -f1)
echo "First DatabaseTickWaits reference file: $file"
rg -n "sentMessageIDs" -S src/IMessage/Sources/IMessage/$file || trueRepository: beeper/platform-imessage
Length of output: 793
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' src/IMessage/Sources/IMessage/DatabaseTickWaits.swift
sed -n '140,260p' src/IMessage/Sources/IMessage/DatabaseTickWaits.swiftRepository: beeper/platform-imessage
Length of output: 7171
Synthetic chat fallback is too narrowly gated by quotedMessageID == "".
In pkg/connector/handlematrix.go, the createChatFromSyntheticPortal retry runs for any SendText error only when quotedMessageID == "". This has two problems:
- For non-synthetic portals,
createChatFromSyntheticPortalwill fail withportal ID does not contain synthetic iMessage recipients, masking the originalSendTexterror. - If there’s a reply/quote (
quotedMessageID != ""), the fallback won’t run even when the portal thread is synthetic and thread creation could be what’s needed.
Gate the fallback based on whether the portal is actually a synthetic portal (or on an error condition indicating an invalid/missing thread), not solely on quote presence.
🤖 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/handlematrix.go` around lines 31 - 76, The fallback to
createChatFromSyntheticPortal in HandleMatrixMessage is currently only allowed
when quotedMessageID == "" which prevents retries for synthetic portals and
hides original SendText errors; change the condition so the retry runs when the
portal is actually synthetic or when SendText returns an error that indicates a
missing/invalid thread. Concretely: replace the quotedMessageID == "" check
around the SendText error path with a check like
isSyntheticPortal(string(msg.Portal.ID)) (or implement a helper method on Client
to detect synthetic portals from msg.Portal.ID/Name) OR inspect the returned
error for a thread-not-found/invalid-thread sentinel and call
c.createChatFromSyntheticPortal(string(msg.Portal.ID), msg.Content.Body,
msg.Portal.Name) in those cases (still passing quotedMessageID as-is). Ensure
this change affects the SendText error branch in HandleMatrixMessage and keep
other logic (media handling and unsupported types) unchanged.
| loginID := imessageid.MakeUserLoginID(currentUser.ID) | ||
| remoteName := currentUser.DisplayText | ||
| if remoteName == "" { | ||
| remoteName = currentUser.Email | ||
| } | ||
| if remoteName == "" { | ||
| remoteName = currentUser.PhoneNumber | ||
| } | ||
| if remoteName == "" { | ||
| remoteName = currentUser.ID | ||
| } | ||
|
|
||
| ul, err := l.User.NewLogin(ctx, &database.UserLogin{ | ||
| ID: loginID, | ||
| RemoteName: remoteName, |
There was a problem hiding this comment.
Validate the local account ID before building loginID.
On Line 53, currentUser.ID is used without an emptiness check. If it is empty, login creation can fail later with a less actionable error.
Suggested fix
- loginID := imessageid.MakeUserLoginID(currentUser.ID)
+ if currentUser.ID == "" {
+ return nil, fmt.Errorf("local iMessage account ID is empty")
+ }
+ loginID := imessageid.MakeUserLoginID(currentUser.ID)📝 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.
| loginID := imessageid.MakeUserLoginID(currentUser.ID) | |
| remoteName := currentUser.DisplayText | |
| if remoteName == "" { | |
| remoteName = currentUser.Email | |
| } | |
| if remoteName == "" { | |
| remoteName = currentUser.PhoneNumber | |
| } | |
| if remoteName == "" { | |
| remoteName = currentUser.ID | |
| } | |
| ul, err := l.User.NewLogin(ctx, &database.UserLogin{ | |
| ID: loginID, | |
| RemoteName: remoteName, | |
| if currentUser.ID == "" { | |
| return nil, fmt.Errorf("local iMessage account ID is empty") | |
| } | |
| loginID := imessageid.MakeUserLoginID(currentUser.ID) | |
| remoteName := currentUser.DisplayText | |
| if remoteName == "" { | |
| remoteName = currentUser.Email | |
| } | |
| if remoteName == "" { | |
| remoteName = currentUser.PhoneNumber | |
| } | |
| if remoteName == "" { | |
| remoteName = currentUser.ID | |
| } | |
| ul, err := l.User.NewLogin(ctx, &database.UserLogin{ | |
| ID: loginID, | |
| RemoteName: remoteName, |
🤖 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/login.go` around lines 53 - 67, Before calling
imessageid.MakeUserLoginID with currentUser.ID, validate that currentUser.ID is
non-empty and return a clear error (or handle appropriately) if it is empty;
update the code around creation of loginID so it checks currentUser.ID, logs or
returns an error early, and only calls imessageid.MakeUserLoginID and
l.User.NewLogin when the ID is present (references: currentUser.ID, loginID,
imessageid.MakeUserLoginID, l.User.NewLogin, database.UserLogin).
| package imessage | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
|
|
||
| "github.com/beeper/platform-imessage/pkg/imessage/lib" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== client.go header =="
sed -n '1,12p' pkg/imessage/client.go
echo
echo "== pkg/imessage/lib files and headers =="
fd -t f . pkg/imessage/lib | sort | while read -r f; do
echo "--- $f ---"
sed -n '1,12p' "$f"
doneRepository: beeper/platform-imessage
Length of output: 677
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== list pkg/imessage/lib/* =="
fd -t f pkg/imessage/lib | sort
echo
echo "== go:build lines in pkg/imessage/lib/* =="
for f in $(fd -t f pkg/imessage/lib | sort); do
# print only top build tag lines (if any), without dumping entire files
echo "--- $f ---"
rg -n --max-count 5 '^//go:build ' "$f" || true
rg -n --max-count 5 '^// \+build ' "$f" || true
done
echo
echo "== check symbols referenced by pkg/imessage/client.go from pkg/imessage/lib =="
# Grab identifiers used as `lib.<Ident>` in client.go
rg -n --no-heading 'lib\.[A-Za-z0-9_]+' pkg/imessage/client.go | sed -E 's/.*lib\.([A-Za-z0-9_]+).*/\1/' | sort -uRepository: beeper/platform-imessage
Length of output: 486
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== list pkg/imessage/lib files =="
fd -t f . pkg/imessage/lib | sort
echo
echo "== go:build lines in pkg/imessage/lib/* (top few matches) =="
for f in $(fd -t f . pkg/imessage/lib | sort); do
echo "--- $f ---"
rg -n --max-count 10 '^(//go:build|// \+build )' "$f" || true
done
echo
echo "== check symbols referenced by pkg/imessage/client.go from pkg/imessage/lib =="
rg -n --no-heading 'lib\.[A-Za-z0-9_]+' pkg/imessage/client.go | sed -E 's/.*lib\.([A-Za-z0-9_]+).*/\1/' | sort -uRepository: beeper/platform-imessage
Length of output: 558
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== list pkg/imessage/*.go =="
ls -1 pkg/imessage/*.go 2>/dev/null || true
echo
echo "== go:build lines in pkg/imessage/*.go (top few matches) =="
for f in pkg/imessage/*.go; do
[ -f "$f" ] || continue
echo "--- $f ---"
rg -n --max-count 20 '^(//go:build|// \+build )' "$f" || true
done
echo
echo "== any go:build lines inside pkg/imessage/client.go (entire file, limited) =="
rg -n '^(//go:build|// \+build )' pkg/imessage/client.go || trueRepository: beeper/platform-imessage
Length of output: 343
Add darwin/cgo build constraints to pkg/imessage (or stubs for pkg/imessage/lib)
pkg/imessage/client.go has no build tags but imports pkg/imessage/lib; that package only contains lib_darwin.go with //go:build darwin && cgo, so non-darwin (and darwin with CGO_ENABLED=0) will fail with “no Go files”/build-constraint errors.
Fix by adding matching darwin && cgo build tags to pkg/imessage/client.go (and any other files that use lib), or by adding non-darwin/!cgo stub implementations in pkg/imessage/lib that return a clear unsupported-platform error.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 7-7: could not import github.com/beeper/platform-imessage/pkg/imessage/lib (-: build constraints exclude all Go files in pkg/imessage/lib)
(typecheck)
🤖 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/imessage/client.go` around lines 1 - 8, The package imports
github.com/beeper/platform-imessage/pkg/imessage/lib which currently only has
lib_darwin.go with //go:build darwin && cgo, causing build-constraint errors on
other platforms; either add the same build tag //go:build darwin && cgo to
pkg/imessage/client.go (and any other files importing lib) so those files are
only built on darwin+cgo, or create non-darwin/!cgo stub files in package
imessage/lib (e.g., lib_nondarwin.go) that provide the same exported symbols
used by client.go but return a clear unsupported-platform error (refer to the
import line and any functions/methods in client.go that call into lib, and
mirror their signatures in the stubs).
| return c.IM.DeleteChat(string(msg.Portal.ID)) | ||
| } | ||
|
|
||
| func (c *Client) makeOwnReactionID(reactionKey string) networkid.EmojiID { |
There was a problem hiding this comment.
Self-reaction dedup key won't match the iMessage echo: This synthesizes EmojiID = c.GetUserID() + reactionKey, but the upsert path in handleReactionStateSync keeps EmojiID = reaction.ID, and Swift sets Reaction.ID = ${participantID}${reactionKey} (see PlatformSDKMessage.swift:50-55). c.GetUserID() is the bridge login ID, which is not guaranteed to equal Swift's self-participant ID (Swift typically returns the normalized email/phone for participantID). When the iMessage echo arrives, the EmojiID won't match the Matrix-side row and the reaction will be duplicated (and Tapback toggles won't clean up). Either normalize using Swift's participant ID for self, or rewrite the upsert EmojiID via makeOwnReactionID when reaction.ParticipantID is self.
| message.ThreadID = evt.ObjectIDs.ThreadID | ||
| } | ||
| message := message | ||
| c.queueMessageActionChange(context.Background(), message) |
There was a problem hiding this comment.
Action messages get double-emitted: queueMessageActionChange emits a ChatInfoChange for handled action types, but execution falls through and the same message is queued as a normal simplevent.Message. For thread_participants_added/_removed/thread_title_updated/group_thread_created the action message body is empty, so convertMessageFromIMessage falls into the len(converted.Parts) == 0 branch (messageconv.go:41-47) and emits a text event with body "Unsupported iMessage event" alongside the member change. Have queueMessageActionChange signal that it handled the event and continue the loop here so action-only messages aren't double-posted.
| timeout = 0 | ||
| } | ||
| meta := c.baseEventMeta(evt.ThreadID).WithType(bridgev2.RemoteEventTyping) | ||
| meta.Sender = bridgev2.EventSender{Sender: imessageid.MakeUserID(evt.ParticipantID)} |
There was a problem hiding this comment.
Self typing is rebroadcast as a ghost: meta.Sender is built unconditionally from evt.ParticipantID. If Swift ever emits a user_activity event for the local user (typing in Messages.app on this Mac is a plausible source), Matrix will see a typing event from a ghost user instead of from self — looks like a stranger is typing in the user's own DM. Either filter out self events here or set EventSender{IsFromMe: true, SenderLogin: c.UserLogin.ID, Sender: c.GetUserID()} when the participant matches self.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/connector/chatinfo.go (1)
150-156:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSkip empty participant IDs when building chat members.
chatInfoFromThreadcurrently inserts members even whenparticipant.IDis empty, which can create invalid sender identities in the member map.Suggested fix
memberMap := bridgev2.ChatMemberMap{} for _, participant := range thread.Participants.Items { + if participant.ID == "" { + continue + } userID := imessageid.MakeUserID(participant.ID) memberMap.Set(bridgev2.ChatMember{ EventSender: bridgev2.EventSender{Sender: userID}, Membership: event.MembershipJoin, UserInfo: c.userInfoFromUser(participant), }) }🤖 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/chatinfo.go` around lines 150 - 156, In chatInfoFromThread skip participants with empty IDs before creating a sender and inserting into memberMap: check participant.ID == "" (or its equivalent) and continue/skip so you don't call imessageid.MakeUserID or memberMap.Set for empty IDs; update the loop that constructs bridgev2.ChatMember (which currently calls imessageid.MakeUserID(participant.ID) and c.userInfoFromUser(participant) and then memberMap.Set) to only perform those actions when participant.ID is non-empty.
🧹 Nitpick comments (4)
pkg/imessage/types.go (4)
35-40: 💤 Low valueConsider adding a validation method for
ThreadType.While the two exported constants (
ThreadTypeSingle,ThreadTypeGroup) define valid values, there's no validation preventing invalidThreadTypevalues. Adding a simpleIsValid()method would help catch mistakes if these values are ever constructed manually (though typically they're unmarshaled from Swift JSON).Optional validation helper
+// IsValid returns true if the ThreadType is one of the defined constants. +func (t ThreadType) IsValid() bool { + return t == ThreadTypeSingle || t == ThreadTypeGroup +}🤖 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/imessage/types.go` around lines 35 - 40, Add a validation helper for the ThreadType enum so callers can verify values before use: implement a method named IsValid() on the ThreadType type that returns bool and checks the receiver against the allowed constants ThreadTypeSingle and ThreadTypeGroup; update any unmarshaling or input paths (where ThreadType values are constructed or parsed) to call ThreadType.IsValid() and handle invalid values accordingly (e.g., return an error or default).
24-33: ⚡ Quick winDocument the empty-string return convention in
Pagination.JSON().The method returns
("", nil)when pagination is absent (nil receiver or empty cursor/direction), requiring callers to check for an empty string. This is a valid but non-obvious design choice that should be documented in a godoc comment explaining when""vs. a JSON string is returned.Consider adding a comment like:
// JSON returns a JSON representation of the pagination parameters. // Returns ("", nil) if the receiver is nil or if cursor or direction is empty. // Otherwise returns the JSON-encoded string or a marshaling error.🤖 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/imessage/types.go` around lines 24 - 33, Add a godoc comment for the Pagination.JSON method that documents its empty-string convention: explain that Pagination.JSON returns ("", nil) when the receiver is nil or when Cursor or Direction is empty, and otherwise returns the JSON-encoded string or an error from json.Marshal; update the comment immediately above the func (p *Pagination) JSON() to reference Pagination.JSON, Cursor, and Direction so callers know when to expect "" vs a JSON string.
5-174: ⚡ Quick winAdd godoc comments to all exported types and key fields.
The package exports 17 types (
CurrentUser,Page,Pagination,ThreadType,Thread,User,Message,MessageAction,TextAttributes,TextEntity,MentionedUser,MessageLink,Attachment,Asset,Reaction,StateSyncEvent,StateSyncIDs) without any documentation. Adding concise godoc comments improves API discoverability and clarifies the Swift↔Go contract, especially for fields likeThread.MutedUntil,Message.Seen, andStateSyncEvent.Entrieswhere the type or purpose may not be obvious.Example:
// CurrentUser represents the authenticated iMessage user. type CurrentUser struct { // ID is the unique identifier for the user. ID string `json:"id"` // DisplayText is the user's display name. DisplayText string `json:"displayText,omitempty"` // ... }🤖 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/imessage/types.go` around lines 5 - 174, Add concise godoc comments for every exported type and their key exported fields in this file (e.g., CurrentUser, Page, Pagination, ThreadType, Thread, User, Message, MessageAction, TextAttributes, TextEntity, MentionedUser, MessageLink, Attachment, Asset, Reaction, StateSyncEvent, StateSyncIDs). For each type add a one-line comment that starts with the type name and for non-obvious fields (e.g., Thread.MutedUntil, Message.Seen, StateSyncEvent.Entries, Thread.Participants, Message.Action) add brief field comments explaining purpose and expected values/types; follow Go doc convention (start comment with the type/field name) and place comments immediately above the type/field declarations so the godoc is generated correctly.
50-50: ⚡ Quick winNarrow
Thread.MutedUntilandThread.Extrainstead of usingany
ThreadMapperbuilds these fields with a much more specific contract thananyimplies:mutedUntilis either"forever"ornil, andextrais either{"isSMS": true}ornil. Inpkg/imessage/types.go,Thread.MutedUntil(line 50) andThread.Extra(line 58) could use more specific Go types (e.g.,*string/enum forMutedUntil, and a small struct or typed map forExtra) to avoid losing type safety at the boundary.🤖 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/imessage/types.go` at line 50, Change the loose any types on Thread to concrete types: replace Thread.MutedUntil with a pointer type (e.g., *string or a small enum type) that represents either nil or the literal "forever", and replace Thread.Extra with a small struct (e.g., type ThreadExtra struct { IsSMS *bool } or similar) instead of any; then update ThreadMapper to construct those concrete types (set MutedUntil to nil or pointer to "forever", and Extra to nil or &ThreadExtra{IsSMS: ptr(true)}) so callers get compile-time type safety.
🤖 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/imessage/types.go`:
- Line 82: The Message struct currently uses generic any for Seen and Extra
which loses the Swift-side union/dictionary contract; replace Message.Seen with
a dedicated union type (e.g., type MessageSeen struct or a sum type) that can
represent a boolean, numeric timestamp (int64/float64) or a participants object,
implement UnmarshalJSON on that MessageSeen to inspect json.RawMessage and
decode into the correct variant (bool, timestamp, or participants struct), and
change Message.Extra from any to a JSON object type such as map[string]any (or
json.RawMessage if you prefer deferred decoding); update the Message struct in
pkg/imessage/types.go and add the new MessageSeen type + its UnmarshalJSON so
decoding preserves the original typed semantics.
---
Outside diff comments:
In `@pkg/connector/chatinfo.go`:
- Around line 150-156: In chatInfoFromThread skip participants with empty IDs
before creating a sender and inserting into memberMap: check participant.ID ==
"" (or its equivalent) and continue/skip so you don't call imessageid.MakeUserID
or memberMap.Set for empty IDs; update the loop that constructs
bridgev2.ChatMember (which currently calls imessageid.MakeUserID(participant.ID)
and c.userInfoFromUser(participant) and then memberMap.Set) to only perform
those actions when participant.ID is non-empty.
---
Nitpick comments:
In `@pkg/imessage/types.go`:
- Around line 35-40: Add a validation helper for the ThreadType enum so callers
can verify values before use: implement a method named IsValid() on the
ThreadType type that returns bool and checks the receiver against the allowed
constants ThreadTypeSingle and ThreadTypeGroup; update any unmarshaling or input
paths (where ThreadType values are constructed or parsed) to call
ThreadType.IsValid() and handle invalid values accordingly (e.g., return an
error or default).
- Around line 24-33: Add a godoc comment for the Pagination.JSON method that
documents its empty-string convention: explain that Pagination.JSON returns ("",
nil) when the receiver is nil or when Cursor or Direction is empty, and
otherwise returns the JSON-encoded string or an error from json.Marshal; update
the comment immediately above the func (p *Pagination) JSON() to reference
Pagination.JSON, Cursor, and Direction so callers know when to expect "" vs a
JSON string.
- Around line 5-174: Add concise godoc comments for every exported type and
their key exported fields in this file (e.g., CurrentUser, Page, Pagination,
ThreadType, Thread, User, Message, MessageAction, TextAttributes, TextEntity,
MentionedUser, MessageLink, Attachment, Asset, Reaction, StateSyncEvent,
StateSyncIDs). For each type add a one-line comment that starts with the type
name and for non-obvious fields (e.g., Thread.MutedUntil, Message.Seen,
StateSyncEvent.Entries, Thread.Participants, Message.Action) add brief field
comments explaining purpose and expected values/types; follow Go doc convention
(start comment with the type/field name) and place comments immediately above
the type/field declarations so the godoc is generated correctly.
- Line 50: Change the loose any types on Thread to concrete types: replace
Thread.MutedUntil with a pointer type (e.g., *string or a small enum type) that
represents either nil or the literal "forever", and replace Thread.Extra with a
small struct (e.g., type ThreadExtra struct { IsSMS *bool } or similar) instead
of any; then update ThreadMapper to construct those concrete types (set
MutedUntil to nil or pointer to "forever", and Extra to nil or
&ThreadExtra{IsSMS: ptr(true)}) so callers get compile-time type safety.
🪄 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: a9df7546-9072-4fce-882a-1e39c1ae14d6
📒 Files selected for processing (12)
pkg/connector/chatinfo.gopkg/connector/client.gopkg/connector/commands.gopkg/connector/connector.gopkg/connector/handleimessage.gopkg/connector/handlematrix.gopkg/connector/messageconv.gopkg/imessage/client.gopkg/imessage/lib/lib_darwin.gopkg/imessage/types.gopkg/imessageid/dbmeta.gosrc/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift
✅ Files skipped from review due to trivial changes (1)
- pkg/connector/commands.go
🚧 Files skipped from review as they are similar to previous changes (7)
- pkg/imessageid/dbmeta.go
- pkg/connector/messageconv.go
- pkg/connector/connector.go
- pkg/imessage/lib/lib_darwin.go
- pkg/connector/handlematrix.go
- pkg/imessage/client.go
- src/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-03T17:00:24.877Z
Learnt from: KishanBagaria
Repo: beeper/platform-imessage PR: 69
File: src/IMessage/Sources/IMessage/EventWatcher/EventWatcher+Updates.swift:89-93
Timestamp: 2026-05-03T17:00:24.877Z
Learning: In the `platform-imessage` codebase (Swift), message IDs (`PlatformSDK.MessageID`) are raw GUIDs — they are not hashed or transformed. `message.id` is set directly from `msgRow.guid` in `MessageMapper.swift`, with multi-part messages appending `_<part.index>` as a suffix. There is no GUID→public ID hashing step for messages.
Applied to files:
pkg/connector/handleimessage.go
🔇 Additional comments (2)
pkg/connector/client.go (1)
41-45: Duplicate: bridge can remain in Connected state whenStartEventsfails.Still present and already reported earlier in this PR discussion.
pkg/connector/handleimessage.go (1)
114-116: Duplicate: reaction delete events still miss participant sender attribution.Still present and already reported earlier in this PR discussion.
| Attachments []Attachment `json:"attachments,omitempty"` | ||
| Links []MessageLink `json:"links,omitempty"` | ||
| Reactions []Reaction `json:"reactions,omitempty"` | ||
| Seen any `json:"seen,omitempty"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find Swift code that populates Seen and Extra fields in Message objects
# Search for Message JSON encoding or Seen/Extra assignment
rg -iC5 --type=swift '"seen".*:' -g '!**/test/**'
rg -iC5 --type=swift 'extra.*Message|Message.*extra' -g '!**/test/**'Repository: beeper/platform-imessage
Length of output: 15393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Read the relevant section in pkg/imessage/types.go
ls -la pkg/imessage/types.go
sed -n '60,110p' pkg/imessage/types.go | cat -n
# 2) Find the exact declarations for Seen/Extra and any custom json decoding helpers nearby
rg -n "Seen|Extra" pkg/imessage/types.go
# 3) Search for where seen/extra are decoded/mapped on the Go side (likely json decoding into these fields)
rg -n '"seen"|seen|\"extra\"|extra' pkg/imessage -S || trueRepository: beeper/platform-imessage
Length of output: 3293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find Message decoding/unmarshal logic in Go
rg -n --hidden --no-ignore-vcs "json\.Unmarshal|Decoder\(" pkg/imessage -S || true
rg -n --hidden --no-ignore-vcs "\bUnmarshal\b|Decode\b" pkg/imessage -S || true
# Find any custom handling for "seen" or "extra" fields on the Go side
rg -n '"seen"|\"extra\"|\bseen\b|\bextra\b' pkg/imessage --glob='*.go' -S || true
# Locate constructors/parsers that produce Message values
rg -n --glob='*.go' "func .*Message|type Message|Message\{" pkg/imessage -S || trueRepository: beeper/platform-imessage
Length of output: 4168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# View decode helper + how it unmarshals Message payloads
sed -n '1,140p' pkg/imessage/client.go | cat -n
sed -n '140,320p' pkg/imessage/client.go | cat -n
# Find how Go code uses Message.Seen/Extra (beyond unmarshalling)
rg -n --glob='*.go' "\.seen\b|\.extra\b" pkg/imessage -S
# Find Swift serialization for extra (mirroring seen?.jsonValue)
rg -n --type=swift "seen\?\.jsonValue|extra\?\.jsonValue|\"seen\"\s*:\s*.*jsonValue|\"extra\"\s*:\s*.*jsonValue" src/IMessage -S || true
# Find Swift message model properties/types for seen/extra (heuristic)
rg -n --type=swift "\bvar\s+seen\b|\blet\s+seen\b|\bseen\s*:" src/IMessage -S || true
rg -n --type=swift "\bvar\s+extra\b|\blet\s+extra\b|\bextra\s*:" src/IMessage -S || trueRepository: beeper/platform-imessage
Length of output: 7721
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# View decode helper and how payloads become Message
sed -n '1,140p' pkg/imessage/client.go | cat -n
sed -n '140,320p' pkg/imessage/client.go | cat -n
# Find how Go code uses Message.Seen/Extra
rg -n --glob='*.go' "\.seen\b|\.extra\b" pkg/imessage -S || true
# Swift: look for how extra is encoded (mirroring seen?.jsonValue)
rg -n --type=swift "seen\?\.jsonValue|extra\?\.jsonValue|\"seen\"\s*:\s*.*jsonValue|\"extra\"\s*:\s*.*jsonValue" src/IMessage -S || true
# Swift: try to locate message model fields for seen/extra
rg -n --type=swift "\b(var|let)\s+seen\b|\bseen\s*:" src/IMessage -S || true
rg -n --type=swift "\b(var|let)\s+extra\b|\bextra\s*:" src/IMessage -S || trueRepository: beeper/platform-imessage
Length of output: 10299
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find MessageSeen definition + jsonValue
rg -n --type=swift "MessageSeen\b|jsonValue\b" src/IMessage -S
# Print the relevant definition area for MessageSeen (small slices)
# Use a targeted path if rg yields something; otherwise print around first match.Repository: beeper/platform-imessage
Length of output: 2825
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show MessageSeen enum content
sed -n '40,160p' src/IMessage/Sources/PlatformSDK/PlatformSDKMessage.swift | cat -n
# Show message object seen/extra declarations
sed -n '1,120p' src/IMessage/Sources/PlatformSDK/PlatformSDKMessageObject.swift | cat -n
# Show JSON.swift protocols for jsonValue types (if needed)
sed -n '1,120p' src/IMessage/Sources/PlatformSDK/JSON.swift | cat -nRepository: beeper/platform-imessage
Length of output: 7541
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect Swift MessageSeen definition (cases + jsonValue typing)
sed -n '40,160p' src/IMessage/Sources/PlatformSDK/PlatformSDKMessage.swift | cat -n
# 2) Inspect Swift message object declarations for seen/extra
sed -n '1,120p' src/IMessage/Sources/PlatformSDK/PlatformSDKMessageObject.swift | cat -n
# 3) Inspect Swift JSON protocols/types used by jsonValue
sed -n '1,120p' src/IMessage/Sources/PlatformSDK/JSON.swift | cat -nRepository: beeper/platform-imessage
Length of output: 7541
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/IMessage/Sources/IMessage/Mappers/MessageDraft.swift | cat -n
rg -n --type=swift "\bvar\s+extra\b|extra:\s*JSONObject|\bextra\s*=\s*JSONObject|extra:\s*Any\?" src/IMessage/Sources/IMessage/Mappers src/IMessage/Sources/PlatformSDK -S || trueRepository: beeper/platform-imessage
Length of output: 4721
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/IMessage/Sources/IMessage/Mappers/MessageDraft.swift | cat -n
rg -n --type=swift "\bvar\s+extra\b|extra:\s*JSONObject|\bextra\s*=\s*JSONObject|public\s+let\s+extra:\s*Any\?" src/IMessage/Sources/IMessage/Mappers src/IMessage/Sources/PlatformSDK -S || trueRepository: beeper/platform-imessage
Length of output: 4721
Refine Message.Seen/Message.Extra away from any in pkg/imessage/types.go (lines 82 and 92)
Seenis not arbitrary on the Swift side:PlatformSDK.MessageSeenis a typed union (bool,timestamp,participants) whosejsonValueis emitted as a JSON boolean, numeric timestamp, or object—decoding into GoSeen anythrows away that contract; model the union explicitly (or decode viajson.RawMessage+ union parsing).Extrais constructed asJSONObject([String: Any]) inMessageDraft/MessagePatchand passed through asextra(nil when empty); decode to a JSON-object/dictionary type in Go (e.g.,map[string]any) instead ofExtra any.
🤖 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/imessage/types.go` at line 82, The Message struct currently uses generic
any for Seen and Extra which loses the Swift-side union/dictionary contract;
replace Message.Seen with a dedicated union type (e.g., type MessageSeen struct
or a sum type) that can represent a boolean, numeric timestamp (int64/float64)
or a participants object, implement UnmarshalJSON on that MessageSeen to inspect
json.RawMessage and decode into the correct variant (bool, timestamp, or
participants struct), and change Message.Extra from any to a JSON object type
such as map[string]any (or json.RawMessage if you prefer deferred decoding);
update the Message struct in pkg/imessage/types.go and add the new MessageSeen
type + its UnmarshalJSON so decoding preserves the original typed semantics.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/client.go`:
- Around line 70-71: The goroutine started with syncExistingChats() doesn’t
observe the disconnect signal and can keep running after Disconnect(); modify
syncExistingChats (and any similar long-running loops referenced around the same
area) to accept the stop signal used by eventLoop (e.g., accept the
stopEventLoop channel or a context.Context parameter), add select checks inside
its polling/processing loops to return when the stop signal is received, and
ensure Disconnect() waits for those goroutines to exit (use the existing
stopEventLoop close or a sync.WaitGroup) so no background work continues after
disconnect.
🪄 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: ada92ac4-ea46-49fa-8ef2-cf55297ea98c
📒 Files selected for processing (7)
pkg/connector/client.gopkg/connector/integration_logic_test.gopkg/connector/login.gopkg/imessage/client.gopkg/imessage/lib/lib_darwin.gopkg/imessage/types.gosrc/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/imessage/types.go
- pkg/imessage/lib/lib_darwin.go
- src/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift
- pkg/imessage/client.go
📜 Review details
🔇 Additional comments (6)
pkg/connector/login.go (2)
105-105: ValidatecurrentUser.IDbefore derivingloginID.
imessageid.MakeUserLoginID(currentUser.ID)still executes without an empty-ID guard, which can fail later with a less actionable error path.
50-97: LGTM!Also applies to: 151-211
pkg/connector/client.go (3)
63-68: Bridge state and login flag are set to success beforeStartEvents()is confirmed.If
StartEvents()fails, the method returns with a connected-looking state.
172-185: Typinguser_activityis always rebroadcast as a remote sender.Self typing events can be emitted as ghost-user typing because sender is always derived from
evt.ParticipantID.
35-53: LGTM!Also applies to: 74-125
pkg/connector/integration_logic_test.go (1)
194-269: LGTM!
| c.UserLogin.Log.Warn().Err(err).Msg("Failed to start iMessage event watcher") | ||
| return | ||
| } | ||
| go c.syncExistingChats() |
There was a problem hiding this comment.
syncExistingChats ignores the disconnect signal: eventLoop now correctly consumes the per-Connect stopEventLoop channel, but syncExistingChats (started right above) doesn't take any stop signal and walks every chat + every message page via blocking Swift calls. After Disconnect, this goroutine keeps running, keeps invoking the Swift bridge, and keeps queuing ChatResync / MarkUnread / message events into the (now-disconnected) UserLogin queue. Plumb the same stop channel in (or a context.Context) and bail out of both loops when it fires.
| } | ||
| for _, permission := range authStatus.Permissions { | ||
| if permission.ID != "accessibility" || permission.Authorized { | ||
| continue |
There was a problem hiding this comment.
Per-send Swift round-trip for accessibility check: HandleMatrixMessage now calls c.IM.AuthorizationStatus() for every single Matrix-side send, which crosses the cgo boundary and runs the Mach-O MacPermissions checks. Accessibility status almost never changes mid-session; cache it (e.g. an atomic.Bool on Client, populated during Connect's existing check) and only re-query on a send failure that suggests the permission was revoked.
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/login.go`:
- Around line 74-75: SubmitUserInput currently ignores the input map so the
"permissions_ready" confirmation is never enforced; update
LocalLogin.SubmitUserInput to read the provided map[string]string, verify that
the key "permissions_ready" is present and set to an accepted confirmation value
(e.g. "true"/"yes"), and return a clear error if missing/incorrect before
proceeding to ensureOnlyLocalLoginOwner and the rest of the flow; apply the same
validation to the other SubmitUserInput implementation that mirrors this logic.
In `@pkg/imessage/types.go`:
- Around line 33-37: The Pagination.JSON() method currently returns an empty
string unless both Cursor and Direction are present, causing limit-only
pagination to be dropped; update the guard in Pagination.JSON() (and the similar
check at lines 45-47) to permit serialization whenever Cursor != "" OR Direction
!= "" OR Limit > 0, and ensure the produced JSON/query includes the Limit field
(respecting omitempty) so {Limit: N} is preserved even if Cursor/Direction are
empty.
🪄 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: 85dfc080-126d-4748-bf77-b23a22c641d7
📒 Files selected for processing (19)
mautrix-imessagepkg/connector/backfill.gopkg/connector/capabilities.gopkg/connector/chatinfo.gopkg/connector/client.gopkg/connector/commands.gopkg/connector/connector.gopkg/connector/example-config.yamlpkg/connector/handleimessage.gopkg/connector/handlematrix.gopkg/connector/integration_logic_test.gopkg/connector/login.gopkg/connector/messageconv.gopkg/imessage/client.gopkg/imessage/lib/lib_darwin.gopkg/imessage/lib/lib_stub.gopkg/imessage/types.gosrc/IMessage/Sources/IMessage/PlatformAPI.swiftsrc/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift
🚧 Files skipped from review as they are similar to previous changes (9)
- pkg/connector/capabilities.go
- pkg/connector/connector.go
- pkg/connector/messageconv.go
- pkg/connector/chatinfo.go
- pkg/imessage/client.go
- pkg/connector/backfill.go
- pkg/connector/client.go
- pkg/imessage/lib/lib_darwin.go
- src/IMessage/Sources/IMessageBridgeKit/IMessageBridgeKit.swift
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-05-03T17:00:19.662Z
Learnt from: KishanBagaria
Repo: beeper/platform-imessage PR: 69
File: src/IMessage/Sources/IMessage/EventWatcher/EventWatcher+Updates.swift:89-93
Timestamp: 2026-05-03T17:00:19.662Z
Learning: In the beeper/platform-imessage Swift codebase, keep message IDs (`PlatformSDK.MessageID`) as raw GUIDs. When mapping from DB/event rows to `message.id`, set the ID directly from `msgRow.guid` (no GUID→public-ID hashing or transformation). For multi-part messages, append the part index as `_<part.index>` to the GUID-derived ID. During code review, if changes touch message ID creation/mapping, ensure this raw GUID + optional `_<part.index>` suffix behavior is preserved.
Applied to files:
src/IMessage/Sources/IMessage/PlatformAPI.swift
📚 Learning: 2026-05-03T17:00:24.877Z
Learnt from: KishanBagaria
Repo: beeper/platform-imessage PR: 69
File: src/IMessage/Sources/IMessage/EventWatcher/EventWatcher+Updates.swift:89-93
Timestamp: 2026-05-03T17:00:24.877Z
Learning: In the `platform-imessage` codebase (Swift), message IDs (`PlatformSDK.MessageID`) are raw GUIDs — they are not hashed or transformed. `message.id` is set directly from `msgRow.guid` in `MessageMapper.swift`, with multi-part messages appending `_<part.index>` as a suffix. There is no GUID→public ID hashing step for messages.
Applied to files:
pkg/connector/handleimessage.gopkg/connector/handlematrix.go
🔇 Additional comments (13)
src/IMessage/Sources/IMessage/PlatformAPI.swift (1)
210-214: LGTM!Also applies to: 221-223
pkg/imessage/lib/lib_stub.go (1)
1-120: LGTM!pkg/connector/example-config.yaml (1)
1-4: LGTM!pkg/connector/login.go (1)
19-20: LGTM!Also applies to: 48-49, 105-112, 157-167, 175-175, 196-204, 221-227
pkg/connector/handleimessage.go (2)
31-42: Still valid: action-only state-sync messages are still emitted twice.
queueMessageActionChangecan enqueue theChatInfoChange, but this loop still always queues the same payload as a normalsimplevent.Messageafterwards. For action messages without body content, that still produces the fallback"Unsupported iMessage event"message alongside the real chat update.
62-66: LGTM!Also applies to: 86-113, 141-151, 158-209
pkg/connector/handlematrix.go (5)
48-50: Still valid: the synthetic-chat retry is still too broadly/narrowly gated.This branch still retries on any
SendTexterror as long as there's no quote. That can still hide the original send failure on real portals, and quoted sends on synthetic portals still skip the retry entirely.
68-80: Still valid: only the first returned iMessage ID is persisted.If Swift returns multiple message IDs for one send, only
firstSentMessage(sent)is recorded here. The other echoes can still arrive through state sync as brand-new remote messages and duplicate the user's own send.
218-225: Still valid: outbound reactions still lose the original Matrix timestamp.The stored reaction row is still stamped with
time.Now()instead of the Matrix event time, so history ordering and audit data can still drift from the source event.
320-322: Still valid: self-reaction IDs still won't reliably match the iMessage echo.
makeOwnReactionIDis still derived fromc.GetUserID(), but inbound iMessage reaction IDs are keyed by the iMessage participant ID. When those identities differ, the echo still won't line up with the local DB row.
79-80: LGTM!Also applies to: 121-126, 338-376
pkg/connector/commands.go (1)
97-144: LGTM!Also applies to: 184-199
pkg/connector/integration_logic_test.go (1)
72-147: LGTM!Also applies to: 223-225, 247-432, 440-470, 584-598
| func (l *LocalLogin) SubmitUserInput(ctx context.Context, _ map[string]string) (*bridgev2.LoginStep, error) { | ||
| if err := l.ensureOnlyLocalLoginOwner(ctx); err != nil { |
There was a problem hiding this comment.
SubmitUserInput does not validate the confirmation field.
The step defines permissions_ready, but submit ignores the map, so confirmation is never enforced.
Suggested fix
-func (l *LocalLogin) SubmitUserInput(ctx context.Context, _ map[string]string) (*bridgev2.LoginStep, error) {
+func (l *LocalLogin) SubmitUserInput(ctx context.Context, input map[string]string) (*bridgev2.LoginStep, error) {
+ if _, ok := input[loginFieldConfirm]; !ok {
+ return nil, fmt.Errorf("confirm %q before continuing", loginFieldConfirm)
+ }
if err := l.ensureOnlyLocalLoginOwner(ctx); err != nil {
return nil, err
}Also applies to: 95-95
🤖 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/login.go` around lines 74 - 75, SubmitUserInput currently
ignores the input map so the "permissions_ready" confirmation is never enforced;
update LocalLogin.SubmitUserInput to read the provided map[string]string, verify
that the key "permissions_ready" is present and set to an accepted confirmation
value (e.g. "true"/"yes"), and return a clear error if missing/incorrect before
proceeding to ensureOnlyLocalLoginOwner and the rest of the flow; apply the same
validation to the other SubmitUserInput implementation that mirrors this logic.
| type Pagination struct { | ||
| Cursor string `json:"cursor"` | ||
| Direction string `json:"direction"` | ||
| Limit int `json:"limit,omitempty"` | ||
| } |
There was a problem hiding this comment.
Pagination.JSON() currently drops limit-only pagination requests.
Limit was added, but the current guard returns "" unless both cursor and direction are present, so {limit: N} gets silently ignored.
Suggested minimal fix
func (p *Pagination) JSON() (string, error) {
- if p == nil || p.Cursor == "" || p.Direction == "" {
+ if p == nil || (p.Cursor == "" && p.Direction == "" && p.Limit == 0) {
return "", nil
}
data, err := json.Marshal(p)Also applies to: 45-47
🤖 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/imessage/types.go` around lines 33 - 37, The Pagination.JSON() method
currently returns an empty string unless both Cursor and Direction are present,
causing limit-only pagination to be dropped; update the guard in
Pagination.JSON() (and the similar check at lines 45-47) to permit serialization
whenever Cursor != "" OR Direction != "" OR Limit > 0, and ensure the produced
JSON/query includes the Limit field (respecting omitempty) so {Limit: N} is
preserved even if Cursor/Direction are empty.
| } | ||
|
|
||
| func (p *Pagination) JSON() (string, error) { | ||
| if p == nil || p.Cursor == "" || p.Direction == "" { |
There was a problem hiding this comment.
Limit-only pagination is silently dropped: With Limit now a real field, this guard still requires both Cursor and Direction. A caller passing &Pagination{Limit: N} will get "" back and the limit will never reach Swift. Either weaken the guard to also allow Limit > 0, or document that callers must always supply a cursor+direction (in which case the new Limit field is only useful through backfillPagination).
| helper.Copy(up.Int, "event_poll_timeout_ms") | ||
| helper.Copy(up.Bool, "skip_permission_validation") | ||
| } | ||
|
|
There was a problem hiding this comment.
Default skips all permission validation: ShouldSkipPermissionValidation() returns true when the pointer is nil, and example-config.yaml ships skip_permission_validation: true. With that default, LocalLogin.Start/SubmitUserInput (login.go:53-78) and Connect (client.go:34-65) bypass the entire AuthorizationStatus gate — including IMESSAGE_PERMISSIONS_MISSING / IMESSAGE_PERMISSION_CHECK_FAILED bridge states and the prior RequestAuthorization("all") prompt. A user without Accessibility/Contacts/Messages-Data/Automation now logs in cleanly and only discovers the gap when IM.SendText/IM.MarkRead fails; best-effort MarkRead failures are then silently swallowed by shouldIgnoreBestEffortAutomationError. If this is intentional (so the UI relies on IMESSAGE_AUTOMATION_UNAVAILABLE instead of the legacy missing-permission states), worth a comment on the field; otherwise flip the default.
| return nil, err | ||
| } | ||
| for _, thread := range page.Items { | ||
| if thread.Type != imessage.ThreadTypeGroup && !threadIDIsGroup(thread.ID) { |
There was a problem hiding this comment.
Avoid per-thread cgo for self identifiers: participantSetMatchesThread(expected, thread, c.currentUserIdentifiers()) is evaluated once per thread, and each call to currentUserIdentifiers() runs c.IM.CurrentUser() (a Swift cgo round-trip via IMessageBridgeKit). For a group-create against an account with hundreds of threads this is hundreds of redundant CurrentUser lookups; same pattern in chatInfoFromThread (line ~302) and reIDKnownSyntheticPortals (handleimessage.go:399). Hoist selfIdentifiers := c.currentUserIdentifiers() once before the loop and pass it in.
| if thread.Type != imessage.ThreadTypeGroup && !threadIDIsGroup(thread.ID) { | |
| selfIdentifiers := c.currentUserIdentifiers() | |
| var pagination *imessage.Pagination | |
| for pageCount := 0; pageCount < 200; pageCount++ { | |
| page, err := c.IM.Chats(pagination) | |
| if err != nil { | |
| return nil, err | |
| } | |
| for _, thread := range page.Items { | |
| if thread.Type != imessage.ThreadTypeGroup && !threadIDIsGroup(thread.ID) { | |
| continue | |
| } | |
| if participantSetMatchesThread(expected, thread, selfIdentifiers) { | |
| return &thread, nil | |
| } | |
| } |
| return nil, err | ||
| } | ||
| if existingThread != nil { | ||
| return c.chatResponseFromThread(*existingThread), nil |
There was a problem hiding this comment.
No cancellation in CreateGroup: CreateGroup takes ctx context.Context but never checks it. existingThreadWithParticipants then walks up to 200 pages of IM.Chats(pagination) (each a Swift cgo call) before returning. If the Matrix-side request is cancelled mid-walk we still complete the full scan and may still return a chatResponseFromThread for an already-abandoned request. Add a ctx.Err() check before each c.IM.Chats(...) call (and ideally thread the ctx into the Swift calls when they're made cancellable).
1 participant
Summary
Adds a bridgev2 Matrix bridge entrypoint for iMessage and connects it to the Swift runtime through a new
IMessageBridgeKitdynamic library with a C ABI. The Go bridge includes local login, chat/message sync, live event handling, Matrix send/reply/edit/unsend/reaction/read/typing paths, and identifier/contact helpers.This also documents the bridge build flow in
bridge-readme.md, adds Go module metadata, wires the new Swift product/target intoPackage.swift, and gates SwiftUI previews behindENABLE_SWIFTUI_PREVIEWSso the bridge product can build cleanly.Validation
swift build --product IMessageBridgeKitCGO_LDFLAGS="-L$(swift build --show-bin-path) -Wl,-rpath,$(swift build --show-bin-path)" go test -tags nocrypto ./...CGO_LDFLAGS="-L$(swift build --show-bin-path) -Wl,-rpath,$(swift build --show-bin-path)" go build -tags nocrypto ./cmd/mautrix-imessageNotes: SwiftPM reported an existing unhandled-file warning from the
swift-syntaxcheckout. The documented Go build succeeded with duplicate rpath linker warnings.