Skip to content

feat(feed): mentions + stranger involvement, expand replies, reply backfill, bell reconciliation (#1060 r2) - #1064

Merged
fiddur merged 4 commits into
developfrom
timeline-replies-round2
Aug 24, 2026
Merged

feat(feed): mentions + stranger involvement, expand replies, reply backfill, bell reconciliation (#1060 r2)#1064
fiddur merged 4 commits into
developfrom
timeline-replies-round2

Conversation

@fiddur

@fiddur fiddur commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Round 2 on #1060, from Fredrik's field feedback (with screenshots) + his follow-up question "are mentions of me from strangers also dropped?" (they were):

Why the timeline was still full of replies

Entries ingested before #1061 have no in_reply_to_uri — the reply link was discarded at ingest — so the filter can't see them. Timeline reads now lazily backfill a few unchecked legacy entries per read (AP re-fetch of the object, one attempt, stamped via new reply_checked_at; fresh ingests stamp immediately). His timeline converges clean over a few page loads.

Where the toggle is

timeline_show_replies now also sits on the Feed page next to the Home timeline (it stays in Settings too). Saving invalidates the timeline query so it applies immediately.

Involvement: mentions + strangers

The inbox previously dropped every Note from a non-followee — including replies to your own posts and posts mentioning you. Now a non-followee's Note is admitted iff you're involved: it replies to one of your own (still existing) posts, or carries a Mention of your actor (new mentions_me column/field; author snapshot from the signature-verified sender). Involved posts always show (marked "replied to you" / "mentioned you") and always notify — regardless of the reply setting or the per-follow bells. Everything else from strangers is still dropped.

Expand replies

GET /feed/timeline/:id/replies + MCP get_timeline_replies: a live, bounded snapshot of the post's remote AS2 replies collection — ≤20 replies from ≤15 SSRF-guarded fetches in a 12s budget, authors resolved + memoised, HTML sanitised server-side, nothing stored, partial flag when the thread outruns the budget. Web timeline cards get a Show replies button rendering it inline.

Bells can't lie anymore

On launch, AutoEnablePostNotifications reconciles the Feed page's per-account 🔔 bells with the device: any bell on + no explicit on/off choice ever made → request POST_NOTIFICATIONS and start the poller. An explicit "off" is never overridden; a denial records "off" (no re-prompt loop) and the Account screen's existing blocked-warning takes over.

Tests: 5 new remote-replies unit tests (page walk, URI items, budget/partial, sanitisation, author memoisation), parseReplyInfo/backfill tests, db integration for the mentions filter + backfill helpers + getTimelineEntryById, Android involvement-predicate tests; whole-monorepo check green, activitypub suite 156/156, Android compile+unit green locally.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CwoP1SqJhHgHiEEoEQtUjT

…ckfill, bell reconciliation (#1060 round 2)

Fredrik's follow-ups from using the timeline against Mastodon publishers:

- Legacy replies still showed as top-level cards (ingested before
  in_reply_to_uri existed): timeline reads now lazily backfill a few
  unchecked entries per read (AP re-fetch, one attempt, stamped via new
  reply_checked_at; fresh ingests stamp immediately)
- timeline_show_replies toggle now ALSO lives on the Feed page next to
  the Home timeline (saving invalidates the timeline query)
- Involvement now includes Mentions and works for strangers: the inbox
  admits a non-followee Note when it replies to one of the recipient's
  own (existing) posts or Mentions them (author snapshot from the
  signature-verified sender); new mentions_me column/field keeps such
  posts visible and notifying whatever the reply setting says
- Expand replies: GET /feed/timeline/:id/replies + MCP
  get_timeline_replies fetch a live bounded snapshot of the remote
  replies collection (≤20 replies/≤15 SSRF-guarded fetches/12s,
  sanitised, authors memoised); web cards get a Show replies button
- Android: notifier notifies involvement regardless of the per-follow
  bell set; AutoEnablePostNotifications reconciles active bells with
  the device on launch (requests permission + starts the poller unless
  an explicit off was ever chosen) — bells can no longer silently lie

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CwoP1SqJhHgHiEEoEQtUjT
@fiddur
fiddur marked this pull request as ready for review August 24, 2026 06:01

@fiddur fiddur left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

🛑 Changes required

Solid round 2 overall: the stranger-involvement admission is properly narrow (own-post reply verified against feed_post, or an exact-href Mention, plus the activity.actorId/getActor identity check and the existing noteToTimelineInput host/attribution guards), the reply fetch is genuinely budgeted and SSRF-guarded, getTimelineEntryById is per-user-DB scoped, the route validates the UUID before touching the DB, and the Android involvement rule + docs line up with the backend.

Two things should change before merge (both inline):

  1. The reply backfill destroys correct data. develop already stores in_reply_to_uri (shipped in #1061), and this PR's migration leaves reply_checked_at NULL on all existing rows — including those correctly-stamped ones. backfillReplyLinks then writes parseReplyInfo(null, …) (i.e. in_reply_to_uri = NULL) whenever the re-fetch fails, which is common: post deleted upstream (404), instance in authorized-fetch/secure mode (401 for our unsigned GET), host down, or an HTML body returned for the AP Accept. Those replies then resurface as top-level cards and lose in_reply_to_mine, so "replied to you" markers and the Android involvement notification disappear. The unit test currently asserts the clobbering behaviour, so it needs updating too.

  2. TimelineReply.url is an unvalidated remote string rendered into an href. remote-replies.ts sanitises the reply HTML but passes obj.url straight through, and HomeTimeline.tsx renders <a href={reply.url}>. A javascript: URL from a hostile origin executes in the app origin on click — and thanks to mention-based ingestion, any stranger can now get a card (and therefore a "Show replies" button pointing at their own server) into a user's timeline. An http(s)-only check next to the existing sanitizeRemoteHtml call fixes it for both the REST and MCP consumers.

Non-blocking — fold into a later PR, don't re-roll this one

  • federation.ts: the updated ingestFeedActivity docblock is now orphaned — it sits directly above the new ingestStrangerInvolvement docblock, so two doc comments stack on the stranger helper and ingestFeedActivity has none. It also describes only the reply branch, not the Mention-only one.
  • listReplyUncheckedEntries (WHERE reply_checked_at IS NULL ORDER BY received_at DESC) has no supporting index, and there is no index on received_at at all — the analogous retro-enrich query got idx_timeline_entry_unenriched for exactly this reason (cheap no-work case on every timeline read).
  • MCP get_timeline_replies returns jsonResponse({ error, success: false }) where the rest of the file uses errorResponse(...), and re-inlines the 12_000 budget the router keeps as REPLIES_TIMEOUT_MS.
  • invalidateQueries({ queryKey: ['feed', 'timeline'] }) prefix-matches ['feed','timeline',<id>,'replies'], so toggling the setting refetches every expanded thread from its remote origin.
  • partial is reported true for a thread of exactly 20 replies even when the collection was exhausted; conversely an unreachable origin returns partial: false + no replies, which the web renders as "No replies found on the origin." rather than a fetch failure.
  • resolveAuthor does new URL(actorUri) on a raw attributedTo value; a non-URL string throws and (via the route's .catch) collapses the whole thread to empty.
  • noteMentionsActor and the stranger-admission branch — the security-relevant part of this PR — have no unit tests, and origin?: string on ingestNoteForRecipient silently disables mention detection if a future caller omits it (all current callers pass it, so it could just be required).
  • Involvement now notifies even for an account whose bell the user explicitly turned off. Intended per the docs, just worth being deliberate about — it does override an explicit mute.
  • The comment above AutoEnablePostNotifications() in MainActivity.kt restates the function's own KDoc.

Comment thread apps/backend/src/services/timeline-retro-enrich.ts
Comment thread apps/backend/src/services/activitypub/remote-replies.ts Outdated
…round 1)

- Backstamp migration marks rows that already carry in_reply_to_uri
  (ingested between #1061 and now) as checked, and a backfill fetch that
  yields no usable AS2 object (404, authorized-fetch 401, HTML body,
  host down) only stamps reply_checked_at — never overwrites stored
  reply state (new markTimelineEntryReplyChecked)
- TimelineReply.url is kept only when http(s): a hostile origin —
  reachable by any stranger via mention ingestion — could otherwise put
  a javascript: href in the web's reply list
- resolveAuthor no longer throws on a non-URL attributedTo (it
  collapsed the whole thread to empty via the route's catch)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CwoP1SqJhHgHiEEoEQtUjT

@fiddur fiddur left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

🛑 Changes required

Round 2 fixes from the last round both landed cleanly: the backfill is now no-clobber (markTimelineEntryReplyChecked on a non-answer, plus the timeline_entry_reply_checked_backstamp migration for rows that already carry a reply link — and the unit test asserts the new behaviour), TimelineReply.url is http(s)-only, and resolveAuthor no longer throws on a non-URL attributedTo. The reply walk is genuinely budgeted and SSRF-guarded, getTimelineEntryById is per-user-DB scoped, the route validates the UUID before touching the DB, and the migration keys are non-table names so they re-run on every migrateSchema pass as intended.

Two things should still change before merge (both inline, both one-liners):

  1. A stranger can overwrite a followee's timeline entry. noteToTimelineInput's authority check only enforces attribution when the Note declares attributedTo; with it omitted, any sender may claim any object_uri on their own host. That guard was written for accepted followees ("a malicious accepted followee could deliver a Note whose id collides with another followee's post"), and the stranger branch widens it from "people you chose to follow" to "anyone sharing a host with someone you follow".
  2. TimelineReply.published_at is an unvalidated remote string declared as ISO8601DateTime, and the web feeds it straight to formatDistanceToNow, which throws RangeError: Invalid time value on an unparseable value. There is no error boundary in apps/web, so one malformed published takes the timeline down.

Non-blocking — fold into a later PR, don't re-roll this one

  • federation.ts: the ingestFeedActivity docblock is still orphaned above ingestStrangerInvolvement (two stacked doc comments; ingestFeedActivity has none), and the .on(Create, …) comment now claims "the only stranger Note admitted is a reply to one of the recipient's own posts" — the Mention branch is missing from both, so the comments understate what the code admits.
  • origin?: string on ingestNoteForRecipient is now passed by every caller; making it required would remove the silent "mentions never detected" failure mode for a future caller.
  • Still open from round 1: no index behind listReplyUncheckedEntries (WHERE reply_checked_at IS NULL ORDER BY received_at DESC, run on every timeline read); MCP get_timeline_replies uses jsonResponse({ error, success: false }) where the rest of the file uses errorResponse(...) and re-inlines 12_000; partial is true for an exhausted thread of exactly 20 and false for an unreachable origin (which the web then shows as "No replies found on the origin"); invalidateQueries({ queryKey: ['feed', 'timeline'] }) prefix-matches the per-entry replies queries.
  • Android: when the permission request is denied, setPostNotificationsEnabled(context, false) records an explicit "off", and the Account screen's warning only fires on toggle-on + permission-off — so a user with bells on who denies once gets neither notifications nor the warning, which is the silent-bell case this reconciliation exists to remove.

Comment thread apps/backend/src/services/activitypub/federation.ts
Comment thread apps/backend/src/services/activitypub/remote-replies.ts Outdated
… timestamps (#1064 round 2)

- Stranger involvement branch now requires the Note to explicitly
  attribute to the signing actor — without attributedTo, anyone on a
  followee's host could claim an existing entry's object_uri and
  overwrite it via the upsert (and the actor-scoped Delete would then
  never match)
- TimelineReply.published_at kept only when it parses: the schema
  promises ISO-or-null and date-fns throws on Invalid Date, which
  would blank the page on a hostile published value

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CwoP1SqJhHgHiEEoEQtUjT

@fiddur fiddur left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

🛑 Changes required

Solid round overall. The stranger-admission tightening holds up under scrutiny: explicit attributedTo == signing actor, the existing-own-post lookup, and the note.id.host === senderHost guard inherited from noteToTimelineInput together close the object_uri-overwrite path I went looking for. The no-clobber backfill is right too (markChecked vs saveReplyInfo, plus the idempotent backstamp so any row that already carries in_reply_to_uri is never a candidate — which also means every candidate has a NULL link, so a null result can't clobber anything). The listTimelineEntries filter change is safe (mentions_me is NOT NULL DEFAULT false, no three-valued logic), /timeline/:id/replies is registered ahead of /:postId, the web PATCHes settings partially so the Feed toggle can't wipe other settings, and the ['userSettings'] key matches the Settings page.

Blocking: one authenticity hole in the new reply fetcher — see the inline comment on remote-replies.ts.

Non-blocking — fold into a later PR, don't re-roll this one

  • Two doc comments now document the wrong symbol. timeline-ingest.ts:84 — the noteToTimelineInput "Authority check" block is now attached to export type TimelineAuthor. federation.ts:203-221 — the ingestFeedActivity block ("Two admissible senders…") is stacked directly above ingestStrangerInvolvement, which has its own block, so the text is duplicated and ingestFeedActivity (line 262) is left undocumented.
  • No index for the backfill candidate query. listReplyUncheckedEntries is WHERE reply_checked_at IS NULL ORDER BY received_at DESC LIMIT 3 and runs on every timeline read, but the only index is (published_at DESC, id DESC) — so it is a seq scan + top-N sort, worst exactly once the backlog drains. #996 added idx_timeline_entry_unenriched as a partial index for precisely this shape; a (received_at DESC) WHERE reply_checked_at IS NULL partial index would match that precedent.
  • fetchRemoteReplies returns partial: false when the post itself couldn't be fetched (budget.exhausted is still false), so an unreachable or authorized-fetch origin renders as "No replies found on the origin." — indistinguishable from a genuinely empty thread. Separately, partial is true whenever exactly MAX_REPLIES were collected, even when the collection ended there.
  • isoOrNull accepts anything Date.parse takes, but timelineReplySchema.published_at is z.iso.datetime(); normalising via new Date(v).toISOString() would make the DTO match its own schema.
  • get_timeline_replies returns jsonResponse({ error: 'Not found', success: false }) where every other tool in feed-tools.ts uses errorResponse(...).
  • ${origin}/users/${user} is now hand-built in three places (federation.ts:186, federation.ts:237, api.ts:405) next to the existing ownObjectPrefix helper; an ownActorUri(origin, user) beside it would stop them drifting. Relatedly, ingestNoteForRecipient's origin? is optional but both callers now pass it, so the origin == null → mentions_me = false branch is dead.
  • Product note, not a defect: involvement now notifies regardless of the per-account bell, so a followee whose bell was explicitly switched off will still raise a notification when they reply to or mention you. docs/features/feed.md states this, so I'm reading it as intentional.

Comment thread apps/backend/src/services/activitypub/remote-replies.ts
A reply's byline came from remote-supplied attributedTo with no
authority check, so a hostile origin (reachable by any stranger via
mention ingestion) could render arbitrary text under any fediverse
identity — a clean impersonation/phishing primitive that sanitisation
can't touch.

Replies are now held to the host that actually served them: inline
items inherit the collection page's serving host, URI-referenced items
their own URI's host (with the fetched document's id checked against
it), and both the object's id and its attributedTo must live on that
host or the reply is dropped — the same authority model
noteToTimelineInput enforces on ingest. Tests cover the forged-byline
inline item, the forged-id inline item, and the cross-host URI item.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CwoP1SqJhHgHiEEoEQtUjT

@fiddur fiddur left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ Approved

Read the whole diff plus the surrounding ingest / SSRF / schema-migration code. Round 3 closes the earlier rounds' issues cleanly and I found no blocking correctness or security problem.

What I checked and am satisfied with:

  • Stranger admission (ingestStrangerInvolvement): gated on reply-to-an-existing-own-post (prefix + UUID + getFeedPostById) or an explicit Mention of the recipient's actor URI; requires explicit attributedTo matching the signature-verified activity.actorId, and re-verifies the fetched actor's id. Combined with noteToTimelineInput's host check, the object_uri upsert key can't be hijacked to overwrite another actor's entry.
  • remote-replies.ts: the serving-host authority model (PageCtx.host, inline items inherit the page host, URI items get their own) plus the id/attributedTo host checks in toReply block byline forgery; httpsOnly and isoOrNull sanitize the two values the web feeds to href / date-fns; every fetch goes through safeFetchGet (maxRedirects: 0, private-range blocklist, 1 MB cap) and every failure path is swallowed rather than thrown. The page walk terminates in every cycle shape I could construct — a repeating next URI drains the fetch budget, and parsed JSON can't self-reference.
  • Migrations: timeline_entry_mentions / timeline_entry_reply_checked / ..._backstamp are keys with no matching table name, so they run on the existing-DB path (!existingTableNames.has(tableName)) as well as on fresh init, in the right order relative to timeline_entry_reply and the indexes. The backstamp is genuinely idempotent because the upsert now always stamps reply_checked_at.
  • No-clobber backfill: candidates are reply_checked_at IS NULL, which after the backstamp implies in_reply_to_uri IS NULL, and a non-answer only calls markChecked — the new test asserts exactly that split.
  • Web: dangerouslySetInnerHTML is fed only server-sanitized HTML; the ['userSettings'] and ['feed','timeline'] query keys match the rest of the app; updateUserSettings is a PATCH, so the single-field write doesn't clobber other settings.
  • Android: decideNotifications' new involved || (top-level && bell on) rule is what the docs and the new test describe; fetchFollowingList swallows all exceptions, so AutoEnablePostNotifications' LaunchedEffect can't crash the app when offline, and the launcher is registered before the effect runs.

Non-blocking — fold into a later PR, don't re-roll this one

Please do not push fixes for these here; another round costs more than they're worth. Batch them with other work:

  1. Two orphaned/stale doc comments. In federation.ts the old ingestFeedActivity block ("Ingest a Create/Update … Two admissible senders …") now sits directly above the new ingestStrangerInvolvement doc block, so ingestFeedActivity itself is undocumented and the stacked block reads as describing the stranger helper. It is also now incomplete: it lists only "a reply to one of the recipient's own posts" and omits the Mention branch, as does the .on(Create, …) comment ("the only stranger Note admitted is a reply to one of the recipient's own posts"). Same shape in timeline-ingest.ts, where the noteToTimelineInput doc block (authority check / feed_following presentation) is now attached to the newly inserted TimelineAuthor type. Stale prose sitting on security-critical admission logic is the kind that misleads a later reader.
  2. A couple of redundant comments. // An inline item is held to the page's serving host; a URI item to its own. in collectPageReplies restates what resolvePage's and toReply's docs already say, and // Either way the choice is now recorded; only a grant starts the poller. in PostNotificationAutoEnable.kt narrates the two lines under it. (The {/* Sanitised server-side … */} note in HomeTimeline.tsx is the opposite — keep that one.)
  3. partial over-reports. It is true whenever replies.length >= MAX_REPLIES, so a thread of exactly 20 is reported as truncated, and an author lookup that exhausts the fetch budget marks the whole snapshot partial even when every reply was collected. Conversely, a post whose own fetch fails returns partial: false, replies: [], which the web renders as the confident "No replies found on the origin." rather than as a failure.
  4. Weak assertion in the budget test. expect(replies.length).toBeLessThanOrEqual(20) in "caps at the reply budget and reports partial" would pass with zero replies; toBe(20) would actually pin the cap the test name claims.
  5. Small consistency items. MCP get_timeline_replies returns jsonResponse({ error: 'Not found', success: false }) for not-found while every other not-found in feed-tools.ts uses errorResponse(...); the 12 s budget is REPLIES_TIMEOUT_MS in feed-router.ts but a bare 12_000 literal in the MCP tool; and the reader's own actor URI is now built inline in three places (ingestNoteForRecipient, ingestStrangerInvolvement, api.ts) as ${origin}/users/${user} while ownObjectPrefix uses encodeURIComponent(user) — harmless today given isValidUsername, but a one-line ownActorUri helper next to ownObjectPrefix would stop them drifting.
  6. No unit test for the stranger-admission branch. noteMentionsActor and ingestStrangerInvolvement are the highest-risk new logic and are covered only indirectly. timeline-ingest.test.ts already builds fedify Notes, so "Mention of me admits / Mention of someone else doesn't / missing attributedTo drops" would be cheap insurance.

One design point worth being deliberate about rather than a defect: invalidating ['feed','timeline'] after the settings toggle prefix-matches every ['feed','timeline',<id>,'replies'] query, so each expanded thread refetches — up to 15 outbound requests apiece — on a checkbox flip.

@fiddur
fiddur merged commit 824b973 into develop Aug 24, 2026
4 checks passed
@fiddur
fiddur deleted the timeline-replies-round2 branch August 24, 2026 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant