Repository navigation
push: add support for registering fcm pushers - #26
Merged
Merged
Conversation
tulir
reviewed
Sep 12, 2026
| _, err := ReadProtoResponse[*gvproto.RespRegisterDestination]( | ||
| c.MakeRequest(ctx, http.MethodPost, APIBaseURL+"/api2notifications/registerdestination", nil, http.Header{ | ||
| "Content-Type": {ContentTypeProtobuf}, | ||
| "X-Gv-Rpc-Id": {"1639"}, |
Member
There was a problem hiding this comment.
Is this meant to be hardcoded? It sounds like it might be a request transaction id rather than a method identifier
Also this should probably just be in client.go with the rest of the methods, since there's no other push handling in libgv
Comment on lines
+156
to
+204
| config := gc.Main.Bridge.Config.Backfill | ||
| backfill := &gvBackfillData{ | ||
| thread: thread, | ||
| expected: config.Enabled && (config.MaxInitialMessages > 0 || config.MaxCatchupMessages > 0), | ||
| } | ||
| result := gc.Main.Bridge.QueueRemoteEvent(gc.UserLogin, &simplevent.ChatResync{ | ||
| EventMeta: simplevent.EventMeta{ | ||
| Type: bridgev2.RemoteEventChatResync, | ||
| PortalKey: portalKey, | ||
| CreatePortal: true, | ||
| }, | ||
| ChatInfo: gc.wrapChatInfo(ctx, thread), | ||
| LatestMessageTS: lastMessageTS, | ||
| BundledBackfillData: thread, | ||
| BundledBackfillData: backfill, | ||
| GetChatInfoFunc: func(ctx context.Context, portal *bridgev2.Portal) (*bridgev2.ChatInfo, error) { | ||
| if portal.MXID == "" { | ||
| backfill.expected = config.Enabled && !gc.Main.Bridge.Background && config.MaxInitialMessages > 0 | ||
| } | ||
| return gc.wrapChatInfo(ctx, thread), nil | ||
| }, | ||
| CheckNeedsBackfillFunc: func(_ context.Context, latest *database.Message) (bool, error) { | ||
| limit := config.MaxCatchupMessages | ||
| if latest == nil { | ||
| limit = config.MaxInitialMessages | ||
| } | ||
| backfill.expected = limit > 0 && (latest == nil || lastMessageTS.After(latest.Timestamp)) | ||
| return backfill.expected, nil | ||
| }, | ||
| }) | ||
| continue | ||
| if !result.Success || result.Error != nil { | ||
| if result.Error != nil { | ||
| errs = append(errs, fmt.Errorf("failed to resync chat %s: %w", thread.ID, result.Error)) | ||
| } else { | ||
| errs = append(errs, fmt.Errorf("failed to resync chat %s", thread.ID)) | ||
| } | ||
| continue | ||
| } | ||
| if result.Queued { | ||
| gc.lastEvents[thread.ID] = lastMessageTS | ||
| continue | ||
| } | ||
| if backfill.err != nil { | ||
| errs = append(errs, fmt.Errorf("failed to backfill chat %s: %w", thread.ID, backfill.err)) | ||
| continue | ||
| } else if backfill.expected && !backfill.completed { | ||
| errs = append(errs, fmt.Errorf("backfill did not complete for chat %s", thread.ID)) | ||
| continue | ||
| } | ||
| if backfill.messages != nil { | ||
| messages = backfill.messages | ||
| } |
Member
There was a problem hiding this comment.
This part looks very sus. Why is it backfilling messages and then continuing to handle them as new messages? Different behavior for queued vs not is also unusual
| } | ||
| for _, msg := range thread.Messages { | ||
| for i := len(messages) - 1; i >= 0; i-- { | ||
| msg := messages[i] |
Member
There was a problem hiding this comment.
I assume this was always bridging in the wrong order? slices.Backward is better than manual iteration
Comment on lines
+233
to
+237
| if result.Error != nil { | ||
| errs = append(errs, fmt.Errorf("failed to handle message %s in chat %s: %w", msg.ID, thread.ID, result.Error)) | ||
| } else { | ||
| errs = append(errs, fmt.Errorf("failed to handle message %s in chat %s", msg.ID, thread.ID)) | ||
| } |
Member
There was a problem hiding this comment.
I think we should always be attaching errors to non-successes
Suggested change
| if result.Error != nil { | |
| errs = append(errs, fmt.Errorf("failed to handle message %s in chat %s: %w", msg.ID, thread.ID, result.Error)) | |
| } else { | |
| errs = append(errs, fmt.Errorf("failed to handle message %s in chat %s", msg.ID, thread.ID)) | |
| } | |
| errs = append(errs, fmt.Errorf("failed to handle message %s in chat %s: %w", msg.ID, thread.ID, result.Error)) |
tulir
approved these changes
Sep 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Registers Google Voice's FCM destination and syncs the pushed conversation when Beeper wakes the on-device bridge, including conversations outside the recent thread list. Uses the existing cookie login and client push flow.
Keeps failed syncs eligible for retry. Catch-up checks backfill completion and processes messages in order before advancing its saved position. Buffered cloud events retain their existing queue semantics.
Verified on a Pixel 6: incoming notifications with the bridge stopped, recovery after a missed push, and duplicate suppression after process restart. Cookie-only provider registration, Go build, vet and CI also pass. No client changes.
Device checks used a controlled relay. The shared relay changes in beeper/hungryserv#588 and beeper/push-receiver#4 have now merged.
Preserves the push-device ID across relogin using the existing metadata merge hook, so refreshing credentials does not create another destination identity. The six-line fix builds successfully; it does not change the login flow.
Fresh iOS onboarding remains unresolved: Google accepted push registration, but the last ordinary-login test received no FCM event. The downstream iOS notification path passed with saved Chrome-session credentials.