Skip to content

fix: backfill not working in specific circumstances#182

Merged
highesttt merged 4 commits into
mainfrom
backfill-speed-improvements
Jun 12, 2026
Merged

fix: backfill not working in specific circumstances#182
highesttt merged 4 commits into
mainfrom
backfill-speed-improvements

Conversation

@highesttt

Copy link
Copy Markdown
Collaborator

No description provided.

@indent

indent Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor
PR Summary

Fixes unblock backfill failing in scenarios where bridgev2's silent forward backfill via FetchMessages doesn't repopulate the restored DM portal, catches unblocks that happened while the SSE was down by diffing the cached blockedUsers set against the server's list during fullSync, and (after the follow-up commit) also catches unblocks that happened while the bridge process was offline by persisting the blocked-MIDs list to UserLoginMetadata. The unblock fallback now re-fires queueDMChatResync (silent path via FetchMessages) rather than the live notifying backfillRecentMessages path, so the safety-net no longer risks notifying.

  • refreshBlockedContacts returns (newlyUnblocked []string, error) computed under cacheMu against the union of the in-memory cache and the persisted UserLoginMetadata.BlockedMIDs snapshot.
  • UserLoginMetadata gains a BlockedMIDs field; saveBlockedContacts / saveBlockedContactsSnapshot persist it on every refresh and on every OpBlockContact/OpUnblockContact.
  • New queueUnblockedDMRestore(mid, reason) consolidates the three callers (Connect startup, pollLoop fullSync diff, OpUnblockContact): skips chat MIDs via the existing isChatMID, queues a DM ChatResync, and schedules a 10s queueUnblockBackfillFallback.
  • queueUnblockBackfillFallback now re-queues a queueDMChatResync(true, true) instead of calling backfillRecentMessages, so the safety-net runs through the silent FetchMessages path.
  • The duplicate isLineGroupOrRoomMID helper was removed in favor of the existing isChatMID.
  • backfillRecentMessages retains the token-refresh recovery and debug summary log added earlier (now only used by prefetchMessages).

Issues

1 potential issue found:

  • Doc comment on backfillRecentMessages (sync.go:398-404) is now malformed — the suggested replacement was appended to the original, leaving a dangling "...if bridgev2's silent forward backfill didn't" followed by a second duplicate "backfillRecentMessages fetches..." sentence. The whole block needs to be collapsed back to the intended 3 lines. → Autofix
4 issues already resolved
  • Unblocks that happened while the bridge was offline aren't detected on cold start (latent, triggers on bridge restart following an unblock). Connect discards newlyUnblocked and lc.blockedUsers starts empty, so the first refreshBlockedContacts diff is empty by construction; missed-while-offline unblocks won't queue a DM resync or fallback backfill until a server fullSync event later arrives. Worth confirming whether the restart path is in scope of this fix. (fixed by commit 34714d1)
  • New isLineGroupOrRoomMID duplicates the existing isChatMID helper a few hundred lines below in the same file (sync.go:723). Consider reusing isChatMID (it adds an empty-mid guard which is a harmless no-op at both new call-sites) to avoid future drift. (fixed by commit 34714d1)
  • On cold start, if refreshBlockedContacts fails (e.g., transient LINE API outage), lc.blockedUsers stays empty and the four sync goroutines that fire immediately after will treat previously-blocked DMs as unblocked — syncDMChats queues a ChatResync for them (sync.go:146) and prefetchMessages backfills their messages via the live path (sync.go:382), re-bridging chats the user explicitly blocked. Seed lc.blockedUsers from the persisted UserLoginMetadata.BlockedMIDs snapshot before calling refreshBlockedContacts so the last-known block state survives a failed refresh. (fixed by commit 6601c18)
  • Unblock fallback can fire Matrix notifications for old messages (latent, triggers when bridgev2's silent FetchMessages backfill hasn't persisted to the DB within the 10s unblockBackfillFallbackDelay). The fallback routes through queueIncomingMessage — the live notifying path — so any of the 50 newest messages not yet deduped will bridge as RemoteEventMessages, defeating the silent-on-unblock intent. Consider either reusing the silent FetchMessages/BackfillMessage path or bumping the delay / coordinating with portal-ready signal. (fixed by commit 34714d1)

CI Checks

All CI checks passed on 6601c18.


⚡ Autofix All Issues

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 036a578f-d90c-4316-9226-4bec46ad80ce

📥 Commits

Reviewing files that changed from the base of the PR and between 31d0ed0 and 6601c18.

📒 Files selected for processing (1)
  • pkg/connector/client.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/connector/client.go
📜 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). (2)
  • GitHub Check: build-docker
  • GitHub Check: build-docker

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling when fetching blocked contacts—continues gracefully on fetch errors and persists blocked-contact snapshots on block/unblock events.
  • New Features

    • Automatic DM resync/backfill when contacts are unblocked (immediate resync plus delayed fallback).
    • Retries recent-message backfill after token refresh/recovery.
    • Login metadata now stores blocked-contact IDs to improve restore on startup.

Walkthrough

Detects newly unblocked LINE contacts by diffing refreshed blocked lists vs cached+metadata, persists blocked-contact snapshots, queues immediate DM resyncs and a delayed backfill fallback per unblocked MID, and adds retry/metrics to recent-message backfill.

Changes

Unblock Recovery Workflow

Layer / File(s) Summary
Blocked-MIDs metadata, imports, and refreshBlockedContacts
pkg/connector/connector.go, pkg/connector/sync.go
Adds BlockedMIDs []string to UserLoginMetadata, adds sort import, introduces unblockBackfillFallbackDelay, and changes refreshBlockedContacts to return ([]string, error) while diffing fresh blocked sets against cached+metadata, updating cache under lock, and persisting a sorted snapshot.
Queue helpers: immediate resync + delayed fallback
pkg/connector/sync.go
Introduces queueUnblockedDMRestore to skip group/room MIDs and queue immediate DM resync plus queueUnblockBackfillFallback which schedules a delayed recheck and backfill if still unblocked.
Backfill retry logic and metrics
pkg/connector/sync.go
backfillRecentMessages retries on token-refresh/logged-out fetch failures, tracks queued and skippedExisting counters, and logs fetched/queued/skipped totals with duration.
SSE fullSync and block/unblock handlers wiring
pkg/connector/sync.go
fullSync consumes newlyUnblocked and queues unblocked DM restores; OpBlockContact persists blocked snapshot after update; OpUnblockContact saves snapshot and calls queueUnblockedDMRestore to restore DM behavior.
Client connection call site
pkg/connector/client.go
LineClient.Connect pre-seeds blocked cache from metadata, handles the new two-value result from refreshBlockedContacts, enqueuing unblocked DM restores on success and preserving warn-and-continue behavior on error.

Sequence Diagram(s):

sequenceDiagram
  participant SSE as SSE fullSync
  participant Refresh as refreshBlockedContacts
  participant Cache as lc.blockedUsers
  participant Queue as queueUnblockedDMRestore
  participant Portal as queueDMChatResync
  participant Fallback as queueUnblockBackfillFallback

  SSE->>Refresh: request fresh blocked list
  Refresh->>Cache: diff & update cache\ncompute newlyUnblocked
  Refresh-->>SSE: ([]string newlyUnblocked, error)
  SSE->>Queue: for each newlyUnblocked MID -> queueUnblockedDMRestore(mid, reason)
  Queue->>Portal: queue immediate DM chat resync (portal create + silent backfill)
  Queue->>Fallback: schedule delayed recheck/backfill
  Fallback->>Portal: after delay, if still unblocked -> queue DM chat resync
Loading

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

  • beeper/line#181: Directly related refactor of refreshBlockedContacts signature and the DM resync/backfill workflow integration used in this PR's unblock handler logic.
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'fix: backfill not working in specific circumstances' is vague and does not clearly specify what the actual fix is or which circumstances are involved. Provide a more specific title that describes the actual changes, such as 'fix: restore DM backfill on unblocking contacts at startup' or similar.
Description check ❓ Inconclusive No pull request description was provided, making it impossible to assess whether it relates to the changeset. Add a pull request description that explains the changes, why they were made, and how they address the issue.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch backfill-speed-improvements

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread pkg/connector/sync.go Outdated
Comment thread pkg/connector/client.go Outdated
Comment thread pkg/connector/sync.go Outdated
- unblocks while bridge was offline were not detected

- unblocks could trigger unwanted notifications

- removed duplicate function
Comment thread pkg/connector/sync.go Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 281-286: Seed lc.blockedUsers from the persisted snapshot
(UserLoginMetadata.BlockedMIDs) before calling lc.refreshBlockedContacts so that
syncDMChats and prefetchMessages see the last-known block state if the refresh
fails; then call lc.refreshBlockedContacts and only when it succeeds iterate
newlyUnblocked to call lc.queueUnblockedDMRestore(ctx, mid, "startup"). In
short: populate lc.blockedUsers from lc.UserLogin.Metadata.BlockedMIDs first,
call lc.refreshBlockedContacts next, and avoid queuing restores when refresh
returns an error.
🪄 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: e79feb4a-31d6-44b4-9cdf-8f4ebf023629

📥 Commits

Reviewing files that changed from the base of the PR and between f8e0d68 and 34714d1.

📒 Files selected for processing (3)
  • pkg/connector/client.go
  • pkg/connector/connector.go
  • pkg/connector/sync.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). (2)
  • GitHub Check: build-docker
  • GitHub Check: build-docker
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Use go fmt for code formatting across all Go files
Use goimports with -local "github.com/highesttt/matrix-line-messenger" flag to group project-local imports correctly
Use zerolog for logging throughout the codebase
Do not use Msgf in logging; use Msg with structured fields instead
Use Stringer interface where applicable in Go code

Files:

  • pkg/connector/connector.go
  • pkg/connector/client.go
  • pkg/connector/sync.go
**/!(ltsm)/**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/!(ltsm)/**/*.go: Run staticcheck on all Go files excluding pkg/ltsm package (transpiled WASM code)
Run go vet on all Go files excluding pkg/ltsm package (transpiled WASM code)

Files:

  • pkg/connector/connector.go
  • pkg/connector/client.go
  • pkg/connector/sync.go
pkg/connector/connector.go

📄 CodeRabbit inference engine (AGENTS.md)

Implement bridgev2.NetworkConnector and bridgev2.NetworkAPI interfaces in the connector package for bridge logic

Files:

  • pkg/connector/connector.go
🔇 Additional comments (2)
pkg/connector/connector.go (1)

101-114: LGTM!

pkg/connector/sync.go (1)

33-112: LGTM!

Also applies to: 209-248, 882-887, 971-972, 995-1002

Comment thread pkg/connector/client.go
Comment thread pkg/connector/client.go
highesttt and others added 2 commits June 11, 2026 18:58
Co-authored-by: indent[bot] <216979840+indent[bot]@users.noreply.github.com>
@highesttt highesttt merged commit 710e9e4 into main Jun 12, 2026
10 checks passed
@highesttt highesttt deleted the backfill-speed-improvements branch June 12, 2026 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant