Repository navigation
feat(activity): show reactions that omit the viewer's p tag - #3115
bostonaholic wants to merge 4 commits into
Conversation
Slice 3 of relay-derived reaction recipients
(PLANS/RELAY_DERIVED_REACTION_RECIPIENTS.md): buzz-app admits untagged
reactions to the viewer's messages into Activity.
The reaction feed no longer requires a literal ["p", viewer] tag. A
reaction is a candidate when it has no p tag or its p tags include the
viewer; one whose p tags name only others is dropped before any target
fetch, because every channel reaction reaches the feed's receive().
Target verification is unchanged: the fetched message must be authored
by the viewer, carry one h tag and sit in a member channel, so a forged
p tag on someone else's message still yields no row. The feed now
retains only reactions that passed that check, so foreign reactions
leave after one load and cannot crowd the viewer's rows out of the
100-candidate window across loads.
Known gap: a single burst of more than 100 untagged foreign reactions
can still crowd one load, because the window is sliced before target
verification; the next refresh restores the rows.
The history read stays {kinds:[7], "#p":[viewer], limit: 50}; untagged
reactions appear there once the relay indexes the target's author.
Co-authored-by: mboston Implementer <932d3b6ebea6cc83e091551ae46d1c7e7f8f7d0fa8e9cef5e6facc8544af413b@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Matthew Boston <[email protected]>
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Every client in a channel received each untagged reaction to another member's message and reloaded Activity to check it again. A bounded set of targets verified as foreign now drops those reactions without a read. Unreadable targets stay out of the set so access can return. Co-authored-by: mboston Implementer <932d3b6ebea6cc83e091551ae46d1c7e7f8f7d0fa8e9cef5e6facc8544af413b@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Matthew Boston <[email protected]>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee181fd2b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| items.sort((a, b) => b.createdAt - a.createdAt || a.id.localeCompare(b.id)); | ||
| known = new Set(evidence.map((event) => event.id)); | ||
| retained = reactions; | ||
| retained = relevant; |
There was a problem hiding this comment.
If a channel is archived while Activity retains more than 50 reactions, a refresh now permanently drops its older candidates: relevant excludes every reaction whose target fails allowed(...), so this assignment empties their retained evidence. On unarchive, the restoration refresh can recover only the latest 50 events from history, losing previously displayed reactions from the 100-candidate window. This also affects explicitly recipient-tagged reactions on an updated relay, not just the acknowledged pre-rollout gap for untagged reactions. Drop candidates confirmed to target someone else's message, but preserve bounded evidence for owned or unresolved targets that are temporarily unavailable.
Useful? React with 👍 / 👎.
|
🤖 Could you add a screenshot or short video of the change to the PR? Design uses these for faster reviews. |
…y-untagged-reactions * origin/main: (324 commits) fix(github): show private-repo body images from signed links (#3252) NEXUS-4029: Preserve semantic chips in the core composer (#2303) Make plugin signature enforcement opt-in (#2212) NEXUS-4156: Add workspace Leads and choose a chat's workspace (#2788) feat(plugins): report conclusively retired update providers (#3263) feat(messages): let attachment renderers open files from the host file card (#2239) feat(charts): bundle a Charts code-block renderer (#3226) Expose native reply grouping through Conversation.ui (#3245) NEXUS-4163: Keep plugin-owned chat deletion in Solo (#2787) NEXUS-4156: Expose Solo Workspaces to plugin regions (#2786) NEXUS-4022: Start region chats in the New Chat editor (#2784) feat(link-previews): unfurl web links below messages by default (#3225) fix(messages): keep names in literal Markdown as written when editing (#2834) feat(plugins): fetch a packaged managed plugin policy from a URL (#2504) fix(goose): enable deferred tool loading for goose agents (#3227) fix(unread): preserve manual intent in captured explicit reads (#3228) NEXUS-4337: Expose retained plugin update reviews (#3075) feat(conversation): add a link preview slot below messages (#3219) feat(plugins): let link previews read a public page's preview fields (#3217) Let sidebar row plugins decorate ordinary Solo chats (#3222) ...
joahg
left a comment
joahg
left a comment
There was a problem hiding this comment.
🤖 Automated review, not an approval. This changes what Activity shows in the Buzz client, so it needs a design or core-team reviewer before merge.
Attribution and access, checked against the code at this head: a reaction only becomes a row if its single e tag resolves to a kind 9/40002/40008 event whose messageAuthor is the viewer, that event has exactly one h tag, and allowed(h) holds (viewer is a member, channel not archived, cached or pending). A reaction's own h tag, if present, must match the target's. A forged p tag therefore buys nothing, and reactions on others' messages or in channels the viewer can't see are dropped. Candidates are deduplicated by event id, so a reaction that arrives both live and via the #p history read isn't double-counted. The history query is unchanged. Memoizing only targets verified as someone else's is sound because the author is signed, and unreadable targets are correctly not memoized.
Remaining concerns, both P2:
- Read cost.
receive()callsrefresh()for every untagged live reaction in any subscribed channel, whether or not Activity has been opened. Each reaction on a not-yet-seen message costs every member's client roughly a 50-event history read plus a target read, and closure reads when the viewer has rows. The 1,000-entry memo caps repeat cost per message, not cost per distinct message. Consider resolving the target's author from locally held channel events first, or deferring untagged candidates until Activity has been requested. retained = relevantis a behavior change for tagged reactions too (see inline).
CI is green and the PR is mergeable. None of the changed files carries a FOUNDATION marker. I did not review the relay-side change this depends on, so I can't say whether the relay admits a reaction without an h tag from a non-member. I also did not run the app or the browser specs.
| items.sort((a, b) => b.createdAt - a.createdAt || a.id.localeCompare(b.id)); | ||
| known = new Set(evidence.map((event) => event.id)); | ||
| retained = reactions; | ||
| retained = relevant; |
There was a problem hiding this comment.
🤖 P2: this used to retain every eligible candidate (reactions); now it keeps only reactions whose target is owned, allowed and in an available channel. If a channel is archived or loses access during a refresh, the viewer's own reactions there are dropped from retained. On restore, only the latest 50 come back from history, so rows from the 100-candidate window are lost, including p-tagged ones. Consider dropping only candidates confirmed to target someone else's message (the foreign set) and keeping those whose target is owned but temporarily unavailable or unresolved. A test that archives a channel with more than 50 retained reactions and then unarchives it would pin this down.
| !!target && | ||
| !foreign.has(target) && | ||
| validReactionContent(event.content) && | ||
| (!recipients.length || recipients.some(([, value]) => value === viewer)) |
There was a problem hiding this comment.
🤖 P2: admitting untagged reactions means receive() (which refreshes unconditionally, even if Activity was never opened) now fires for every reaction in every subscribed channel. The foreign memo helps only after a target has been checked once, so the cost is still one full load() per client per distinct foreign message. Resolving the author from events the client already holds for that channel, or skipping the refresh while requested is false for untagged candidates, would remove most of it. The updated test covers repeat reactions to one message but not many reactions across distinct messages.
…om received messages Retaining only displayed rows dropped the viewer's reactions in a channel archived during a refresh; a restore then recovered only the 50 the history read returns. Retain every candidate except those on messages verified as someone else's. Channel messages reach receive() in the same batches as reactions, so their authors now mark foreign targets without a read. Co-authored-by: mboston Implementer <932d3b6ebea6cc83e091551ae46d1c7e7f8f7d0fa8e9cef5e6facc8544af413b@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Matthew Boston <[email protected]>
joahg
left a comment
joahg
left a comment
There was a problem hiding this comment.
🤖 Re-checked at ee446d1 (one commit on 82bdb72). Both earlier P2s are addressed:
- Read cost:
receive()now records the author of every kind 9/40002/40008 message in the same batches, so a live reaction to another member's message the client already holds costs no target read. The new test (50 received messages, 50 untagged reactions, no extra queries) pins that. Reactions on messages the client never received still cost one read each, bounded by the 1,000-entry memo, which seems an acceptable residual. retainednow keeps every candidate except those on targets verified as someone else's, so the viewer's reactions survive a channel being archived during a refresh, and the restore test covers more than the 50 the history read returns. Display still re-checks ownership, channel eligibility and closure, so retaining unresolved candidates doesn't show anything new.
Learning authors from received messages is safe: a message only marks its own event id as foreign, so it can't hide reactions on the viewer's messages without an id collision. No new findings. The docs match the code.
This still changes what Activity shows, so it needs a design or core-team reviewer to approve; this comment is not an approval. A reviewer has also asked for a screenshot or short video.
Why
block/buzz#9285 makes the relay index each reaction under the reacted-to message's author for
#preads, whether or not the reacting client taggedp. Activity → Reactions (#2638) still drops any reaction without a literal["p", you]tag. Until this PR lands, reactions from agents and other clients that omit the tag won't appear, even after the relay change.What
ptag, or whoseptags include you. It still checks that you authored the reacted-to message, that the message has onehtag, and that it is in a channel you belong to. A forgedpon a reaction to someone else's message still shows nothing.ptags name only other people is dropped before any fetch. Every member's client sees every channel reaction live, so this keeps tagged reactions to other people's messages from costing reads.{kinds:[7], "#p":[you], limit: 50}.docs/inbox.md,docs/relay-queries.md,docs/buzz-help.md.This PR is safe to merge before block/buzz#9285: untagged reactions on your messages then appear live but not from history.
Testing
inbox-reactions.test.ts: 19/19 pass. Each new behavior test fails without the change:["p", you]on someone else's message still does not.pnpm checkpasses.vitest run src/features/relay src/features/inbox: 3,124 passed.inbox.specandinbox-tabs.spec(Chromium) after the memo change: 30/30.inbox.specandinbox-tabs.spec: 46/46, including "Reaction activity turns Unread off and opens the exact message".vitest run: 18,310 passed. 6 failures in unrelated files (timeouts under load, aspawnSyncerror) pass when run alone.Known gaps
🤖 Generated with Claude Code