Skip to content

fix(beeper): store plain text, resume backfilled history, classify link previews - #571

Merged
wesm merged 1 commit into
kenn-io:mainfrom
sweenzor:fix/beeper-html-and-tail-backfill
Aug 10, 2026
Merged

fix(beeper): store plain text, resume backfilled history, classify link previews#571
wesm merged 1 commit into
kenn-io:mainfrom
sweenzor:fix/beeper-html-and-tail-backfill

Conversation

@sweenzor

@sweenzor sweenzor commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Beeper's API returns HTML in a message's text field for some messages and plain text for others, with no field distinguishing them. msgvault stored it verbatim, so message bodies, snippets and the search index carried raw markup — on a large archive roughly 10% of messages held <a href> tags, and target="_blank" swamped searches for the ordinary word "target". Text is now converted when it is HTML and left untouched when it is not, so messages containing < or & survive intact.

A chat whose backfill had completed only ever synced forward. Beeper Desktop keeps filling in a network's older history for hours or weeks after it is linked, and that history arrives with old timestamps — it neither advances the chat's last activity nor falls in the reconcile window, so it was never archived. Completed chats now re-check their oldest end once a day and resume the backfill when Beeper has added more behind them. Syncs report how many chats they reopened.

Media that arrives as a forwarded link preview — an Instagram reel, an x.com post — now records the URL it previews in attachments.attachment_metadata. That distinguishes media a sender composed from a public post they forwarded, which matters because forwarded previews can dominate an archive's bytes while remaining recoverable from their URL. Download behaviour is unchanged; everything is still archived.

Existing archives repair themselves: each source re-derives its stored text and attachment metadata once, on its next sync, and reports what it fixed. To repair on demand instead of waiting, or to finish an interrupted pass:

msgvault repair-derived --source-type beeper
msgvault repair-derived --source-type beeper --identifier instagramgo

It reads the verbatim payload archived with every message, so it needs no provider connection and repairs messages Beeper no longer holds. Only derived columns are rewritten — raw payloads, downloaded media and sync cursors are untouched — so it is idempotent. The pass is registered per source type, so other importers that archive raw payloads can opt in without a new command.

🤖 Generated with Claude Code

@roborev-ci

roborev-ci Bot commented Aug 8, 2026

Copy link
Copy Markdown

roborev: Combined Review (70f80ae)

Changes requested: one High-severity PostgreSQL JSONB issue and two Medium-severity repair consistency issues were found.

High

  • PostgreSQL attachment metadata queries fail
    • Location: internal/store/attachments.go:36; internal/store/messages.go:2789, 2907-2910
    • attachment_metadata is JSONB on PostgreSQL, but the new queries bind plain text and use COALESCE(attachment_metadata, ''). This can break provider attachment reads and metadata writes, preventing Beeper and Discord media persistence.
    • Fix: Use JSONBindExpr() for writes and cast JSONB to text when reading, or compare with IS DISTINCT FROM ?::JSONB. Add PostgreSQL coverage for attachment metadata.

Medium

  • Incomplete re-derivation passes are marked as applied

    • Location: internal/rederive/rederive.go:109-115
    • A pass with Summary.Errors > 0 and a nil error is still recorded as applied. Because Beeper repair reports per-message database failures this way, automatic sync may treat an incomplete repair as finished and never retry it.
    • Fix: Do not update the ledger when the summary contains errors, and propagate those errors into the sync summary.
  • Partial Beeper repair failures can become permanent

    • Location: internal/beeper/repair.go:102-114
    • Body, snippet, and FTS updates are sequential but gated by the body mismatch. If the body update succeeds and a later update fails, reruns see the corrected body and skip the remaining repair.
    • Fix: Update the derived fields atomically, or reconcile snippet and FTS independently of body changes.

Reviewers: 2 done | Synthesis: codex, 13s | Total: 7m14s

@wesm

wesm commented Aug 9, 2026

Copy link
Copy Markdown
Member

looking

@wesm
wesm force-pushed the fix/beeper-html-and-tail-backfill branch from 70f80ae to 8f69b64 Compare August 9, 2026 14:09
@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (8f69b64)

Changes need revision due to two medium-severity issues in Beeper tail scanning.

Medium

  • internal/beeper/importer.go:421 — The tail probe treats every API item absent from messages as new history. Events intentionally excluded from archival—such as reactions, hidden events, and some deletions—can repeatedly reopen and rewrite a chat while falsely incrementing ChatsReopened. Filter out event types skipped by processMessage before checking archived IDs, or track processed event IDs separately.

  • internal/beeper/importer.go:250 — When a tail scan is due, disabling the activity filter sends every quiet completed chat through incremental and reconciliation paths after probing. Even unchanged chats incur multiple unnecessary message requests, and failures can cause the entire scan to repeat. Track tail-only chats and skip incremental and reconciliation work when their tail probe finds no changes.


Reviewers: 2 done | Synthesis: codex, 15s | Total: 10m13s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (671912d)

The change has two medium-severity correctness issues in Beeper’s backfill probing that can leave messages permanently unarchived.

Medium

  • internal/beeper/importer.go:348 — Cursorless completed chats can skip backfilled messages. A completed chat with no cursors makes probeChatTail return false, so the tailOnly fast path exits before the existing cs.Newest == "" recovery runs. An old, initially empty chat therefore never imports backfilled messages whose timestamps do not advance LastActivity.

    • Fix: Do not take the tail-only early return when cs.Newest is empty. Fetch the newest page as the cursorless-chat path previously did, and add coverage where another active chat establishes a newer activity cutoff.
  • internal/beeper/importer.go:455 — Event-only pages can permanently block discovery of older content. The tail probe examines only one page and treats a page containing only reactions, tombstones, or hidden events as unchanged. If it lies between the stored cursor and older content messages, every daily scan stops at that page and the older content is never archived.

    • Fix: Follow returned cursors past event-only pages, with duplicate detection and a reasonable bound, until persisted-message IDs or true exhaustion are reached. Add coverage for a full event-only page followed by an older content message.

Reviewers: 2 done | Synthesis: codex, 15s | Total: 9m56s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (a3286f3)

Medium findings prevent approval: repair may leave stale derived data, and the documented link-preview query is unsupported.

Medium

  • internal/beeper/repair.go:102 — Snippet and FTS repair runs only when body text differs. If the body is correct but the snippet or search index is stale or missing, repair skips them and still records the derivation version as applied. Re-derive each field independently or atomically refresh snippet and FTS even when the body is unchanged; add coverage for an unchanged body with stale snippet/FTS data.

  • docs/usage/beeper.md:141 — The documented link-preview query cannot run through msgvault query because the analytics-cache attachments dataset does not export attachment_metadata. Add it to both attachment cache-export paths and DuckDB compatibility handling, with a query-level test, or document another supported query method.


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

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (c1831cf)

Changes need attention: one medium-severity cache invalidation issue was identified.

Medium

  • Stale analytics cache after re-derivationcmd/msgvault/cmd/repair_derived.go:92, internal/beeper/importer.go:148

    Re-derivation updates cache-visible snippets and attachment metadata without invalidating the analytics cache or incrementing a sync messages_updated counter. As a result, rebuildCacheAfterWrite may treat an existing v18 cache as current and leave repaired data stale indefinitely, particularly if the cache was upgraded before a quiet Beeper sync.

    Suggested fix: Force a full cache rebuild after standalone repairs, and record automatic repair work as sync updates—or introduce a dedicated cache revision so staleness detection observes these changes.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 10m23s

@wesm
wesm force-pushed the fix/beeper-html-and-tail-backfill branch from c1831cf to 7f84fb9 Compare August 10, 2026 00:32
@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (7f84fb9)

Verdict: One medium-severity cache invalidation issue; no material security regressions found.

Medium

  • Stale analytics cache after derived-data repairinternal/beeper/repair.go:107
    Re-derivation modifies existing snippets and attachment metadata without advancing a cache-freshness signal. If the cache is already at schema v19, rebuildCacheAfterWrite may skip rebuilding, leaving DuckDB with stale snippets and NULL link-preview metadata even though the repair ledger is marked complete.

    Suggested fix: Record a cache-visible repair revision/update and force a full rebuild when it changes for both manual and sync-triggered repairs. Add an integration test that builds a current-schema cache, repairs existing rows, and verifies the cached values are refreshed.


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

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (c781e2d)

Potential terminal escape injection introduced by HTML entity decoding.

Medium

  • internal/beeper/mapping.go:44 — Numeric HTML entities in remote formatted messages can decode into literal terminal control bytes, such as ESC/BEL. Because the TUI and show-message output the resulting body without terminal sanitization (internal/tui/view.go:728), a malicious participant could inject OSC/CSI sequences that manipulate clipboard contents, terminal titles, or displayed output. Strip C0/C1 controls and ESC after decoding while preserving intended newlines, and sanitize terminal output with a multiline-safe equivalent of textutil.SanitizeTerminal.

Reviewers: 2 done | Synthesis: codex, 15s | Total: 15m11s

@wesm
wesm force-pushed the fix/beeper-html-and-tail-backfill branch from c781e2d to e0c61fe Compare August 10, 2026 02:55
@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (e0c61fe)

One medium-severity issue should be fixed before merging.

Medium

  • internal/beeper/importer.go:458probeChatTail treats context cancellation as “not reopened.” If cancellation occurs while probing the final tail-only chat, syncChat can return successfully, stamp LastTailScan, and complete the run. An immediate retry then skips tail probing for 24 hours, breaking resumable interruption behavior.

    Fix: When ctx.Err() is non-nil, return and propagate the error through syncChat. Record LastTailScan only after an uninterrupted scan.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 11m14s

@wesm
wesm force-pushed the fix/beeper-html-and-tail-backfill branch from e0c61fe to 8e384f0 Compare August 10, 2026 03:54
@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (8e384f0)

One medium-severity issue should be addressed before merge.

Medium

  • internal/rederive/rederive.go:123 — A successful pass advances derived_data_revision even when zero messages were scanned. Since Beeper runs this before its first sync, adding an empty or new source unnecessarily invalidates the analytics cache and triggers a full rebuild after import.
    • Fix: When MessagesScanned == 0 and there are no errors, record the ledger entry with MarkMigrationApplied without advancing the derived-data revision.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 12m59s

@wesm
wesm force-pushed the fix/beeper-html-and-tail-backfill branch from 8e384f0 to 49649e3 Compare August 10, 2026 04:08
@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (49649e3)

Code review verdict: One medium-severity issue requires attention; no high or critical findings.

Medium

  • internal/beeper/mapping.go:45<mx-reply> is processed by mime.StripHTML, which removes the tags but retains Matrix’s quoted fallback content. As a result, message bodies, snippets, and FTS documents incorrectly include the parent message as part of the reply.
    • Fix: Remove the entire <mx-reply>...</mx-reply> block before generic HTML stripping, and add a regression test for reply fallbacks.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 6m29s

…nk previews

- Keep PostgreSQL attachment metadata and derived repairs retryable.
- Skip unchanged tail-only chats during daily probes.
- Walk cursorless and event-only pages to find older backfills.
- Keep canceled tail scans due so interrupted syncs resume immediately.
- Refresh derived text and analytics link metadata consistently.
- Avoid cache invalidation when a derivation pass scans no messages.
- Invalidate analytics caches after derived-data repairs.
- Exclude Matrix reply fallbacks from bodies, snippets, and search.
- Strip terminal controls from decoded and rendered message text.

Generated with Codex
Co-authored-by: Wes McKinney <wesmckinn+git@gmail.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@wesm
wesm force-pushed the fix/beeper-html-and-tail-backfill branch from 49649e3 to ff42707 Compare August 10, 2026 04:21
@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (ff42707)

No Medium, High, or Critical findings; the reviewed changes are clean at the requested severity threshold.


Reviewers: 2 done | Synthesis: codex, 5s | Total: 5m40s

@wesm
wesm merged commit 8b8d1e0 into kenn-io:main Aug 10, 2026
20 checks passed
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