Skip to content

Add Beeper Desktop chat archiving#458

Open
sweenzor wants to merge 1 commit into
kenn-io:mainfrom
sweenzor:feat/beeper
Open

Add Beeper Desktop chat archiving#458
sweenzor wants to merge 1 commit into
kenn-io:mainfrom
sweenzor:feat/beeper

Conversation

@sweenzor

@sweenzor sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Archives every chat network bridged through Beeper Desktop — WhatsApp, Signal, Telegram, Instagram, LinkedIn, X, Facebook, Matrix — via its local REST API. Supersedes #455 as a single consolidated PR.

Usage

msgvault add-beeper       # access token from Beeper Desktop → Settings → Developer
msgvault sync-beeper      # first run backfills full history; later runs are incremental

Each connected network account becomes its own source (signal, whatsapp, …), so networks stay separately filterable in the TUI and search; all rows share message_type = "beeper".

What's stored

Message text with FTS (voice-note transcriptions included), reactions, replies, mentions, group membership, deletion tombstones (archived content survives network deletions), media in the content-addressed attachment store, and the verbatim API JSON per message (raw_format = beeper_json).

Behavior notes

  • The client is GET-only by construction — the archiver can never write to Beeper.
  • Backfills checkpoint continuously and resume after interruption; --full re-fetches and upserts in place.
  • The live API's pagination quirks (hasMore never goes false; re-served pages at history edges) are handled and regression-tested against recorded behavior.
  • Beeper message IDs are only unique per installation: an anchor probe fails fast after a reinstall/re-index instead of silently duplicating the archive.
  • [beeper] config covers daemon scheduling, per-network include/exclude (e.g. exclude whatsapp when using the native importer), and a media size cap; backfill-beeper-media retries failed downloads.

Store/query changes

  • SetMessageEdited helper (nothing wrote is_edited before) and a same-second tiebreak in GetLastSuccessfulSync.
  • The three attachment-replace methods now share one helper; media metadata lands in the insert.
  • Texts-mode SQL type filters derive from TextMessageTypes instead of five hand-maintained literals, and the TUI snippet fallback applies to all text types (was WhatsApp-only).

Verified against a live Beeper Desktop with 8 accounts: full backfill, incremental re-runs, media download, interrupt/resume.

🤖 Generated with Claude Code

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (222d083)

Medium-risk issues found; no Critical or High findings were reported.

Medium

  • internal/beeper/client.go:87 (reachable from internal/beeper/media.go:110)
    Oversized Beeper attachments can bypass the new media cap as an availability control. persistAttachments checks declared fileSize before download, but if fileSize is absent or underreported, GetAssetBytes calls c.get, which uses an unbounded io.ReadAll(resp.Body). A remote participant can cause the daemon/CLI to allocate an attacker-controlled asset before max_media_mb is enforced.
    Fix: Size-limit asset reads at download time: reject Content-Length over the cap, read with io.LimitReader(resp.Body, maxBytes+1), and return a pending marker for over-cap assets. For larger media, stream to a temp file while hashing instead of returning []byte.

  • internal/beeper/importer.go:659
    Incremental reply linking assumes the parent is already archived. Beeper message pages are handled newest-first, so when both a new reply and its parent are in the same incremental page, SetReplyTo runs before the parent row exists and the link is never retried.
    Fix: Process incremental page items oldest-first, or buffer incremental reply pairs and flush them after the page/chat has been upserted.

  • internal/beeper/importer.go:347
    Resumable backfills can permanently lose reply links. flushReplies runs even when backfillChat stopped early due to --limit or a nonfatal fetch error, so replies to older messages that have not been imported yet are resolved to NULL and discarded.
    Fix: Only flush buffered replies once the chat backfill is complete, or persist/retry unresolved reply pairs across resume runs.

  • internal/beeper/importer.go:235
    If the anchor message disappears but the chat still exists, the importer clears state.Anchor; normal completed chats then only run incremental/reconcile paths, so no replacement anchor is selected and reinstall/reindex protection can remain disabled indefinitely.
    Fix: Re-arm the anchor from any regular message seen during incremental/reconcile, or explicitly choose a replacement anchor before completing a run with a nil anchor.


Reviewers: 2 done | Synthesis: codex, 14s | Total: 13m24s

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (96d17b4)

Beeper sync has one High severity state regression and two Medium severity retry/resource-limit issues to address.

High

  • internal/beeper/importer.go:235: When the anchor message is gone but the chat still exists, the code clears state.Anchor and continues. For already-completed chats, normal incremental sync never calls the backfill path that re-arms the anchor, so completed sync state can be persisted with no anchor. That permanently disables the reinstall/re-index guard and can allow reused Beeper message IDs to overwrite or duplicate archived data.

    Fix: Re-arm the anchor during the same run from an eligible persisted message, or select and verify a replacement from stored messages before completing the sync.

Medium

  • internal/beeper/importer.go:523: Incremental reaction events treat every GetMessage failure as a skipped missing target. For transient API errors, the page cursor can still advance past the reaction event, so reactions to older messages outside the reconcile window may never be retried or archived.

    Fix: Only skip on ErrNotFound; for retryable/context/API errors, fail the page or avoid advancing cs.Newest so the next run retries the event.

  • internal/beeper/media.go:110: max_media_mb is enforced only after GetAssetBytes has read the full asset into memory. Because GetAssetBytes uses io.ReadAll, a large attachment with missing or misleading fileSize metadata can still be fully downloaded and allocated before being rejected, making the configured per-attachment cap ineffective for untrusted media.

    Fix: Add a bounded asset download path that checks Content-Length when present and reads through io.LimitReader(maxBytes+1), treating over-limit assets as pending/too-large without retaining the full body.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 9m24s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the roborev findings in dcd966a, each with a regression test:

  • High — anchor never re-armed: runs now select a replacement anchor from recently active chats before completing, so the reinstall guard can't be persisted disabled.
  • Medium — unbounded asset reads: max_media_mb is enforced during the read (Content-Length check + limited reader); declared fileSize is treated as untrusted.
  • Medium — transient reaction-target failures: only 404 is skipped; other errors leave the cursor un-advanced so the event retries next run.
  • Also from the first review round: reply pairs are always buffered (newest-first pages can carry a reply before its parent) and unfinished backfills persist unresolved pairs in the sync state across resumes.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (dcd966a)

Summary verdict: changes need fixes for two medium-severity durability/data consistency issues; no critical or high findings reported.

Medium

  • Location: internal/beeper/importer.go:440

    • Problem: Mid-chat checkpoints persist the advanced Oldest cursor before buffered reply pairs are copied into cs.PendingReplies at the end of syncChat. If a long backfill is interrupted after one of these checkpoints, the next run resumes past those reply messages and loses their reply links permanently.
    • Fix: Synchronize cc.replies into cs.PendingReplies before every checkpoint inside backfillChat, or make checkpointing flush chat-local pending reply state first.
  • Location: internal/beeper/media.go:72

    • Problem: persistAttachments returns immediately when the current Beeper message has no attachments, so existing Beeper attachment rows or pending markers are never cleared if media is removed or a pending-marker message is later fetched with no attachments.
    • Fix: When media handling is enabled, replace Beeper attachments with an empty ref list and recompute attachment stats instead of returning early.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 11m37s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-2 findings addressed in the latest commit, both with regression tests: mid-chat checkpoints now persist buffered reply pairs together with the advanced cursor (tested across both interrupt paths), and messages whose media was removed at the source clear their stale attachment rows/pending markers on re-persist.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (0572984)

Summary verdict: Medium-risk issues remain around Beeper media backfill error handling and SQLite cursor lifetime.

Medium

  • internal/beeper/media.go:216
    BackfillMedia treats every GetMessage failure as beeper_media_source_gone and returns nil. Transient Beeper outages, auth failures, or other fetch errors can therefore complete “successfully” with misleading zero pending/error counts while markers remain.
    Fix: Only treat ErrNotFound as source-gone; for other fetch errors, record/increment the error and return it so the run fails or clearly remains pending.

  • internal/store/messages.go:2317
    ForEachBeeperPendingAttachmentMessage invokes the callback while the result cursor is still open. That callback performs network I/O and writes attachment rows, holding a SQLite read cursor across writes and risking database is locked errors or long WAL retention.
    Fix: Read pending rows into a slice, close rows, then invoke callbacks, matching the buffered pattern used by similar store iterators.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 11m48s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-3 findings addressed in 77ebf10: BackfillMedia now skips only on 404 (transient fetch failures are recorded as errors and keep their markers — regression-tested), and pending-attachment messages are buffered into a slice before the per-item download/write work instead of holding a read cursor open across it.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (77ebf10)

Medium-risk issues remain; no high or critical findings were reported.

Medium

  • internal/beeper/media.go:222
    BackfillMedia leaves the pending attachment marker in place when the source message returns 404. Later backfills can retry the permanently missing message indefinitely and may report 0 still pending while the marker remains.
    Fix: Clear Beeper-managed attachment rows for that message, recompute attachment stats, or persist a permanent skipped state that excludes it from future pending scans.

  • internal/beeper/participants.go:42
    A cache entry created from a bare Beeper user ID can block later phone/email resolution for the same user ID, causing participant forks instead of cross-source dedupe when richer metadata appears later in the run.
    Fix: Track fallback cache entries and upgrade/merge them when resolveUser later sees phone/email metadata, or pre-seed rich participant metadata before resolving bare sender IDs.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 11m33s

@wesm

wesm commented Jul 8, 2026

Copy link
Copy Markdown
Member

Thank you for working on this, I am not a Beeper user but this is a good opportunity to set it up!

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for working on this, I am not a Beeper user but this is a good opportunity to set it up!

Happy to! I like beeper a lot, good options to run your own bridges, self-host etc. And I generally like breaking open walled-garden chat products.

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-4 findings addressed in the latest commit, both regression-tested: a 404'd source message now clears its pending media marker (no more eternal retries), and bare-ID participant cache entries are upgraded when phone/email metadata appears later in the run, preserving cross-archive dedup.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (f35c30a)

High-level verdict: changes need fixes for Beeper media/anchor data-integrity risks before merge.

High

  • internal/beeper/media.go:221 - BackfillMedia fetches messages by stored chatID / source_message_id without first verifying the Beeper anchor. The main sync protects against Beeper reinstall/re-index because message IDs can be reassigned; this path bypasses that guard and can attach media from a different message to an existing archived row.
    • Fix: Run imp.verifyAnchor(ctx, syncID, state) before processing pending media and abort on mismatch before any GetMessage calls.

Medium

  • internal/beeper/media.go:227 - When a pending-media source message is gone, ReplaceMessageBeeperAttachments(item.MessageID, nil) deletes all Beeper attachment rows for that message, including already-downloaded attachments. A message with one downloaded attachment and one pending marker would lose archived media if the source message disappears before backfill.

    • Fix: Clear only pending Beeper markers for that message, or preserve refs with a non-empty ContentHash when removing stale pending rows.
  • internal/beeper/importer.go:213 - If verifyAnchor drops a lost anchor and no chats are enumerated in that run, rearmAnchor is a no-op and the sync still completes with anchor nil, disabling the reinstall guard for future incremental runs.

    • Fix: After attempting to re-arm, fail/checkpoint instead of completing when a previously anchored state still has no replacement anchor, or re-arm from known synced chat IDs in state.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 9m24s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-5 findings addressed in the latest commit, all regression-tested: backfill-beeper-media now runs the same anchor verification as the main sync before trusting stored message IDs; the gone-message cleanup clears only pending markers and preserves downloaded attachment rows; and anchor re-arming falls back to chats known from prior sync state when a run enumerates none.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (2958e45)

Medium-risk issues found; no Critical or High findings, and Low findings are omitted.

Medium

  • internal/beeper/media.go:233: BackfillMedia can persist a sync state with Anchor == nil after verifyAnchor treats a missing anchor message as ordinary churn, because it marshals and completes that state without re-arming the anchor. Since this media run becomes the newest completed run, later media backfills and the next main sync can run without the Beeper reinstall/message-ID guard.
    Fix: After verifyAnchor, mirror the main import path by re-arming the anchor before marshaling/completing the media run, or fail the run if no replacement anchor can be established.

  • internal/beeper/importer.go:246: When the anchor message returns 404, any error from the follow-up GetChat probe is treated as “chat no longer exists” and reported as a reinstall/re-index. A transient API error, auth error, or Beeper outage during GetChat would incorrectly tell the user to remove and re-add the account.
    Fix: Distinguish errors.Is(cerr, ErrNotFound) from other errors; only report the reinstall/chat-gone error for ErrNotFound, and return a transient verify beeper sync anchor error otherwise.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 14m2s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-6 findings addressed in the latest commit, both regression-tested: backfill-beeper-media re-arms a soft-cleared anchor before persisting its state (it becomes the newest completed baseline), and the anchor chat-probe only reports a reinstall on 404 — transient probe failures now fail the run retryably instead of prompting a remove/re-add.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (375180b)

Sync changes need one medium follow-up; no critical or high findings reported.

Medium

  • cmd/msgvault/cmd/sync_beeper.go:96: sync-beeper stops on the first account error, so one broken Beeper network source prevents all later registered accounts from syncing and also skips the final cache rebuild for any successful earlier account.
    • Fix: Collect per-account errors, continue syncing remaining accounts, rebuild the cache after the loop, then return a joined/aggregated error if any account failed.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 18m21s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-7 finding addressed in the latest commit: the sync-beeper CLI loop now collects per-account errors and continues with the remaining networks, rebuilds the cache regardless, and reports failures aggregated — matching the sync/sync-full convention and the scheduler path.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (9355534)

Summary verdict: Changes are mostly sound, with two medium-severity reliability issues to address before merge.

Medium

  • internal/beeper/importer.go:252

    • Problem: If the anchor message and its chat are gone, sync treats it as a Beeper reinstall/re-index and blocks the account. Deleting or leaving a chat is normal Beeper churn and can affect whichever chat was chosen as the anchor.
    • Fix: Avoid hard-failing on a 404 chat probe alone. Keep multiple stored anchor probes or fall back to verifying another archived message before requiring remove/re-add.
  • internal/beeper/media.go:264

    • Problem: A transient GetMessage failure during media backfill keeps the pending marker but does not increment or recompute AttachmentsPending, so backfill-beeper-media can report 0 still pending while retry markers remain.
    • Fix: Count preserved pending markers in this branch or recompute remaining pending markers after the loop, and surface sum.Errors in the CLI output.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 15m40s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-8 findings addressed in the latest commit: the reinstall guard now keeps up to three anchor probes in distinct chats, so deleting any one anchored chat is ordinary churn while a reinstall (which invalidates all of them) still fails fast — regression-tested for both cases. Transient media-backfill fetch failures now count their kept markers as still-pending and the CLI surfaces the error count.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (6beca12)

Medium issue found: Beeper sync completion can leave persisted diagnostic counters stale.

Medium

  • Location: internal/beeper/importer.go:223; internal/beeper/media.go:279
  • Problem: Completed Beeper runs do not persist final sync counters. checkpoint(false, ...) is throttled, BackfillMedia only checkpoints before processing, and CompleteSync only writes status/cursor, so quick successful runs can show stale or zero messages_processed, messages_added, and errors_count in sync diagnostics.
  • Fix: Force a final checkpoint with the final summary before CompleteSync in both paths, or add a CompleteSync variant that updates counters atomically.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 14m42s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-9 finding addressed in the latest commit: both sync and media-backfill runs force a final checkpoint with their closing counters before CompleteSync, so quick runs no longer show stale/zero counts in sync diagnostics (asserted in the e2e test).

Separately: the red test check is not this PR — govulncheck started flagging two new Go stdlib CVEs (GO-2026-5856, GO-2026-4970) fixed in go1.26.5, with all traces through pre-existing code (imap, api, backup, emlx). Every branch will fail that step until CI's Go is bumped from 1.26.4; happy to send a separate PR for the toolchain bump if useful.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (0823fb7)

Medium finding blocks a clean merge: anchor verification can silently continue with no valid surviving anchors.

Medium

  • internal/beeper/importer.go:271
    If every stored anchor message returns ErrNotFound while the chats still exist, verifyAnchors drops all anchors and allows sync to continue. A Beeper reinstall or re-index could leave chat IDs intact while invalidating message IDs/cursors, which means the importer may trust stale cursors or duplicate/corrupt archived rows instead of stopping as intended.
    Fix: Treat len(kept) == 0 as a hard anchor failure when prior state had anchors, or require at least one surviving verified anchor before using existing cursors and completing the run.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 13m7s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-10 finding addressed in the latest commit: since chat/room IDs survive a reinstall, losing every anchor probe is no longer excused by the chats still existing — recently archived messages now arbitrate. If one resolves at the source with a matching timestamp the losses were churn (e.g. disappearing-message chats); otherwise the run fails as a reinstall before trusting any stored cursor. Covered by the existing churn/reinstall tests plus enriched media fixtures.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (3571495)

Overall verdict: one medium issue should be addressed before merge.

Medium

  • internal/beeper/participants.go:48: Phone/email-resolved Beeper users only map u.ID in the in-memory cache; the Beeper user ID is never persisted on the rich participant. After a cache reset, or for departed/truncated participants seen only as bare sender/reaction IDs, resolveID can create a separate weak participant, splitting contacts and reactions for the same person.
    • Fix: Persist the Beeper user ID as an identifier on the phone/email participant, and merge/repoint any existing weak participant row for that identifier.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 16m21s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-11 finding addressed in the latest commit: phone/email-resolved participants now persist their Beeper user ID as an identifier (re-pointing any existing bare-ID row), so bare sender IDs in later runs resolve to the same person across cache resets — regression-tested in both directions (rich-then-bare and weak-then-upgraded).

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (79f2c91)

Medium issue found; no High or Critical findings. One reviewer reported no security issues.

Medium

  • internal/beeper/importer.go:217: If all anchors are lost, verifyAnchors clears state.Anchors, but rearmAnchors is best-effort and its failure is ignored before completing the run. The next run then skips anchor verification entirely, disabling the reinstall/re-index guard for an account that already has archived messages.
    • Fix: Make re-arming return an error or success flag and avoid completing with zero anchors when archived messages exist, or persist a state marker that forces archived-sample verification until anchors are restored.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 16m18s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-12 finding addressed in the latest commit: verification no longer fast-paths the zero-anchor case — with no probes, recently archived messages are verified instead (no-op for fresh accounts), so the reinstall guard cannot lapse for an account with history even if a prior re-arm failed. Regression test covers both the passing (intact install, re-arms) and blocking (re-assigned IDs) sides.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (0e37f11)

Medium issue remains: Beeper participant upgrades can split identity history.

Medium

  • internal/beeper/participants.go:70 - Upgrading a Beeper user from a bare-ID participant to a phone/email participant only repoints the participant_identifiers row. Existing messages, reactions, mentions, and conversation membership still reference the old weak participant, leaving history split when richer metadata appears later.
    • Fix: When recording a rich resolution, detect the previous Beeper-ID owner and merge or repoint references from the old participant to the rich participant before or while moving the identifier.

Reviewers: 2 done | Synthesis: codex, 5s | Total: 16m57s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Round-13 finding addressed in the latest commit: when a rich (phone/email) resolution discovers the Beeper user ID is owned by a weak bare-ID participant, that participant's messages, reactions, mentions, and membership are merged into the rich row (constraint-safe dedup) and the fork deleted — guarded so a previous owner with real contact metadata is never absorbed. Verified on SQLite and PostgreSQL.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (c49514d)

No Medium, High, or Critical findings were reported.

Both reviews found no actionable findings at Medium severity or above.


Reviewers: 2 done | Synthesis: codex, 5s | Total: 12m18s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto #460 (Go 1.26.5 bump) and latest main, so CI should now be fully green including the govulncheck gate. The go.mod/flake.nix bump commit will drop out of this diff once #460 merges.

@roborev-ci

roborev-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (9edd31a)

Medium issue found; no High or Critical findings.

Medium

  • internal/beeper/participants.go:44: The rich participant cache short-circuits before checking the current record’s phone/email fields, so a user first seen with email and later with an E.164 phone never gets upgraded to the phone-based participant. This violates the phone-first resolution path and can miss cross-source dedupe.
    • Fix: Track resolution strength and allow stronger metadata, especially phone, to upgrade an existing rich cache entry, merging or repointing existing history as needed.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 12m15s

@sweenzor

sweenzor commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main now that #460 is merged (the toolchain commit drops out of this diff), and pushed a holistic cleanup pass over the whole PR — a structural review for readability after the round-by-round fixes, plus the round-15 finding:

  • Round 15 (phone-over-email): rich resolutions now track their rung, so a phone sighting upgrades a cached email-based resolution and folds the email participant into the phone-deduped one; MergeParticipants preserves contact columns on the survivor. Regression-tested both ways (upgrade merges; a contact-bearing previous owner is never absorbed).
  • Structure: package doc; the reinstall guard now lives in anchors.go (with its tests in anchors_test.go) so it reviews as one unit; large functions split along their natural seams (ensureConversation, refreshReactionTarget, persistMentions/persistReactions).
  • Simplifications: the per-anchor GetChat probe is gone (the archived-sample arbiter already decides the no-survivor case with stronger evidence); reply pairs live only in the checkpointed chat state, deleting the mirror machinery; dead exported surface removed.
  • Tests: new coverage for the discovery fallback when an unfinished chat disappears, incremental stuck-cursor liveness, the watermark-hold-on-fetch-error guard, and the media-type mapping table. Package coverage 80.2% → 82.7%; suite verified on SQLite and PostgreSQL.

@roborev-ci

roborev-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (5509418)

Summary verdict: one medium issue should be addressed before merge.

Medium

  • internal/beeper/participants.go:92: Cross-run email-to-phone participant upgrades can split history. A Beeper user first seen with only email gets the Beeper ID attached to the email participant; a later run with phone metadata resolves a phone participant, but hasContact blocks merging the prior email participant, leaving old messages, reactions, and memberships on the stale participant.
    • Fix: Distinguish email-only prior rich rows from conflicting phone/contact owners and merge the email participant into the phone participant for the same Beeper user ID. Add a fresh-resolver cross-run test.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 15m18s

@sweenzor

sweenzor commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Round-16 finding addressed in the latest commit: the merge guard now blocks only on phone-bearing prior owners, so a cross-run email-to-phone upgrade folds the email participant's history into the phone-deduped one (an email-only owner of the same Beeper user ID is the same person seen with weaker metadata). The email rung also never downgrades a phone-resolved identity. Fresh-resolver cross-run test covers the merge, the preserved email, and the no-downgrade direction.

@roborev-ci

roborev-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (bef45c8)

No Medium, High, or Critical issues found.

Review 1 reported only a Low severity finding, which is omitted per instructions. Review 2 found no issues.


Reviewers: 2 done | Synthesis: codex, 6s | Total: 16m53s

@sweenzor sweenzor marked this pull request as ready for review July 9, 2026 00:48
@wesm

wesm commented Jul 10, 2026

Copy link
Copy Markdown
Member

Good stuff. I am going to try to get #464 merged first and then rebase this, bear with me

Archive every chat network bridged through Beeper Desktop via its local REST API, with resumable synchronization, full message fidelity, media backfill, and reinstall safety. This gives msgvault users one durable archive across the chat services connected to their desktop client.

Follow-up refinements folded into this commit:
- Address roborev review: anchor re-arm, bounded media reads, reply/reaction durability
- Address roborev round 2: checkpoint-consistent reply pairs, stale media cleanup
- Address roborev round 3: backfill-media error reporting, buffered pending list
- Address roborev round 4: gone-message marker cleanup, resolver cache upgrade
- Address roborev round 5: backfill anchor guard, precise marker cleanup
- Address roborev round 6: media runs keep the guard armed, probe errors stay transient
- Address roborev round 7: sync-beeper continues past per-account failures
- Address roborev round 8: multi-anchor reinstall guard, honest pending counts
- Address roborev round 9: persist final sync counters on completion
- Address roborev round 10: archived-sample arbiter when no anchor survives
- Address roborev round 11: persist Beeper user IDs on rich participants
- Address roborev round 12: the reinstall guard never lapses on zero anchors
- Address roborev round 13: merge weak participant history on rich upgrade
- Holistic cleanup: reshape for readability, simplify accreted logic, tighten tests
- Address roborev round 16: cross-run email-to-phone upgrades merge history

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Jul 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (d37b9cd)

Incremental Beeper reconciliation is incomplete and may miss edits, deletions, or reaction changes.

Medium

  • internal/beeper/importer.go:197 — Incremental enumeration requests only chats active since the global watermark minus one hour. Chats containing messages within the documented 24-hour reconciliation window may never reach reconcileChat, leaving in-place edits, deletions, and reaction changes undetected. Enumerate chats from the earlier of watermark - 1h and start - reconcileWindow, and add a multi-chat test covering a recently active non-watermark chat changed in place.

Reviewers: 2 done | Synthesis: codex, 6s | Total: 3m43s

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.

2 participants