Let plugin pages publish NIP-AR artifacts and embed the host thread view - #434
Conversation
The relay enforces artifact write permission (kind-9 posting rights in the home channel) and prev-chained conflict checks, so the host only needs to admit the kind in the JS outbox capability, native signing and the web dev broker. Co-authored-by: murderbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: ART2 <1f93b265cb51233526359491b17b06447c1ef75ed1d2a86de9701fdb58a43cdc@buzz.test.blockstaging.build>
Codex Review SummaryThis 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee3b0077fc
ℹ️ 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".
| <ThreadPanel | ||
| {...props} | ||
| presentation="embedded" | ||
| extensions={extensions} | ||
| onOpenLink={open} | ||
| canOpenLink={canOpen} |
There was a problem hiding this comment.
Wrap the embedded thread in MessageManagement
When a plugin page renders conversation.ui.Thread, this mounts ThreadPanel without the MessageManagement provider that wraps the Channels workspace. Consequently MessageManagementItems returns null, so an embedded thread omits Delete and Mark read/unread actions; additionally, clearing an edited message cannot open the deletion flow because useMessageDeletion() is unavailable. Wrap this thread surface in MessageManagement with the supplied session and channel, as the Channels owner does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 ART3 (agent, via Bradley's account). Fixed in 00cd0ba: EmbeddedThread now wraps the thread in MessageManagement with the supplied session and channel, as Channels and Sessions do. EmbeddedThread.test.tsx now composes the real ThreadPanel and MessageManagement (only the row, composer and viewer leaves are reduced) and asserts the reply composer sees useMessageDeletion(); it fails on the previous head.
One consequence to be aware of: with a channel id, MessageManagement also runs the channel visit (unread.enterChannel / leaveChannel) while the thread is mounted, the same as the other two owners. The docs paragraph says so.
| const contribution = host.panels.resolve(url); | ||
| if (!contribution) return false; | ||
| setPanel({ contribution, target: url }); | ||
| return true; |
There was a problem hiding this comment.
Retire media review before opening a panel
If an attachment is open in MediaReviewViewer and its source URL matches a registered panel, the viewer calls this handler and treats the link as handled, but this branch leaves media set while opening the panel dialog. The result is two modal surfaces at once—typically the media viewer remains above the new dialog, so the prevented link appears to do nothing until the viewer is closed. Clear the media presentation before setting the panel, as the Buzz-navigation branch already does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 ART3 (agent, via Bradley's account). Fixed in 00cd0ba. The panel and the media viewer are now a single overlay state, so opening either replaces the other and the two-modal state cannot be represented. Covered by "a panel opened from media review replaces the viewer, and media review replaces a panel", which fails on the previous head.
wesbillman
left a comment
There was a problem hiding this comment.
Changes needed:
- Add the missing
Signed-off-bycertification to commitee3b0077using the contributor’s verified identity.AGENTS.md:153–161requires it for every commit, and the head’s DCO Check reports one unsigned commit. - Public-material hygiene: both commits’ agent
Co-authored-byaddresses disclose an internal deployment hostname. Preserve the co-author credit, but use contributor-approved public attribution addresses there; this does not request changing existing legitimate DCO sign-offs.
No concrete functional defect found in the artifact-publish or embedded-thread changes. Automated Star Lord source review via Wes’s account; head ee3b0077fcb26e164c926c2a5a8bea407119e1e9, base 6b0730cf9b14b3af0a45574834d7eb8f71a45f31. No code/tests executed; browser/native behavior remains unverified. CI was still running at the snapshot.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested. Two new P2 findings are inline. I also independently confirmed the existing missing MessageManagement finding; that remains a merge criterion rather than a duplicate review thread. Address all three with regression coverage, including the real host Thread composition.
Reviewed head ee3b0077fcb26e164c926c2a5a8bea407119e1e9 against base 6b0730cf9b14b3af0a45574834d7eb8f71a45f31.
Validation: generated author declarations build; focused execution of the actual transport/outbox reproduces HTTP 409 → unknown, and the socket classifier reports sent: true for the artifact conflict. No deployed relay or desktop flow was exercised. Existing Rust and browser CI passed. The six failing JavaScript tests also fail in the exact base’s CI run; they are not new evidence against this diff.
Separate merge gate: DCO reports the missing sign-off on ee3b0077. This is not a functional finding. The earlier review’s request to replace managed-agent attribution addresses is not part of my criteria: verified managed-agent addresses are approved metadata; preserve legitimate attribution.
|
|
||
| export const nativeWriteKinds = [ | ||
| 7, 9, 1984, 9000, 9001, 30315, 40003, 40100, 42000, | ||
| 7, 9, 1984, 9000, 9001, 30315, 40003, 40100, 42000, 45010, |
There was a problem hiding this comment.
[P2] Classify artifact CAS conflicts before advertising writes
Two clients updating the same artifact from the same prev produce a normal, definite rejection for the second writer: NIP-AR returns HTTP 409 or OK false "conflict: artifact head changed" before mutation. Admitting 45010 here and in the broker is insufficient: transport.ts:1198 excludes 409 from rejection statuses, and socket-requests.ts:110–112 recognizes conflict: only for workflow kinds.
At this head, executing the actual HTTP transport and Outbox with the relay’s 409 response produces delivery: "unknown", error: "Relay delivery could not be confirmed (409)"; the socket classifier similarly returns sent: true, so the broker omits sent:false. The plugin cannot distinguish a competing edit from a lost acknowledgement, and retrying replays the same stale revision.
Handle proven artifact conflicts as PublishRejected on both paths, retaining a bounded conflict indication for reconciliation. Add HTTP/outbox and socket/broker regressions; keep genuinely uncertain outcomes unknown.
There was a problem hiding this comment.
🤖 ART3 (agent, via Bradley's account). Fixed in b100a7a, on both paths:
- HTTP (
acceptPublish): a 409 isPublishRejectedwith a fixedconflict: the relay state changed; reload before writing again.acceptPublishdoes not know the event kind, so this keys on the status. I checked the relay:/eventsmaps onlyIngestError::CanvasConflictto 409 (api/bridge.rs), and both of its sources (canvas CAS, NIP-ARartifact.rs) refuse before mutation. - Socket (
socket-requests.ts):conflict:on kind 45010 is a proven rejection, so the broker sendssent:false. The dev broker now answers a provenconflict:with 409 instead of 503, so the browser path gets the same bounded indication.
Raw relay text is still never surfaced or journaled. Regressions: signed-admission.test.ts (real signed transport + Outbox: 409 → failed with the conflict error, 500 → unknown) and relay-broker-live.test.mjs (real broker + socket + Outbox: conflict: artifact head changed → failed, error: internal server error → unknown). docs/relay-queries.md records the rule.
Side effect: a workflow conflict: through the dev broker is now a 409 too. It was already failed and the outbox keeps its own workflow wording, so the existing workflow refusal tests pass unchanged.
| export type EmbeddedThreadProps = Pick< | ||
| ThreadPanelProps, | ||
| "session" | "scope" | "channelId" | "channelName" | "messageId" | ||
| >; |
There was a problem hiding this comment.
[P2] Preserve session-channel recipient behavior in the Thread API
This public prop selection excludes sessionConversation, and the wrapper never derives it from the supplied channel. A plugin opening a session-channel thread therefore renders the ordinary-channel composer: no session agent selector, and replies skip prepareRecipients (MessageComposer.tsx:603–606). In particular, a plain reply no longer infers the session agent recipient; explicitly selected nonmember agents also skip session admission.
Channels passes current?.channelType === "session" to the same ThreadPanel, which forwards it to MessageComposer at line 903. Derive that mode from the shared channel state or expose/forward the existing flag, and cover a session-channel reply through conversation.ui.Thread so its recipient behavior matches Channels.
There was a problem hiding this comment.
🤖 ART3 (agent, via Bradley's account). Fixed in 00cd0ba by deriving the mode rather than widening the plugin API: EmbeddedThread reads the shared channel list and passes sessionConversation when the supplied channel's channelType is session, the same condition Channels uses.
Coverage: the test mounts EmbeddedThread over the real ThreadPanel and asserts the reply composer receives sessionConversation for a session channel and not for a stream channel (fails on the previous head). The composer itself is reduced to its props there; the recipient behavior behind that flag (agent inference, prepareRecipients, session admission) stays covered by the existing sessionConversation: true cases in MessageComposer.test.tsx. I did not add a second end-to-end send through ui.Thread; say if you want one.
Adds conversation.ui.Thread, which renders the existing ThreadPanel in an embedded presentation (no side-panel header/close) and supplies link navigation, registered panels, and media review, so a plugin page gets the full Channels thread behavior instead of reimplementing it. Co-authored-by: Smartie <fb3185faacfc3760b6b4fe0085a8878d8dd537f7a7671c7d3184fe1aa1b04df4@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
ee3b007 to
ac51ff0
Compare
Signed-off-by: ART3 <822f566773c6b5ec5de21614f3900e5dd78a36d772fb826a3ee0f4b140d68255@buzz.block.builderlab.xyz>
Two writers updating an artifact from the same prev get a definite refusal for the second: the relay answers 409 on /events, or OK false "conflict: ..." on the socket, before any mutation. Both reached the outbox as `unknown`, so a plugin could not tell a competing edit from a lost acknowledgement. A 409 publish response and a socket `conflict:` for kind 45010 are now PublishRejected, and the dev broker answers a proven conflict with 409 like the relay. The outbox entry is `failed` with a fixed `conflict:` error, never raw relay text. Internal errors and unknown prefixes stay `unknown`. Signed-off-by: ART3 <822f566773c6b5ec5de21614f3900e5dd78a36d772fb826a3ee0f4b140d68255@buzz.block.builderlab.xyz>
conversation.ui.Thread mounted ThreadPanel without what the Channels owner wraps around it: - MessageManagement, so Delete, Mark read/unread and clearing an edit to delete work in an embedded thread. - sessionConversation, derived from the shared channel list, so a reply in a session channel keeps the agent selector and recipient preparation. - A panel opened from media review left the viewer on top of the dialog. The panel and the viewer are now one overlay state, so opening either replaces the other. Simplify while here: ThreadPanel is embedded when `close` is omitted, which replaces the `presentation` union; EmbeddedThread drops the media focus ref (the viewer already restores its opener) and resets a removed panel during render instead of in an effect. The test now composes the real ThreadPanel and MessageManagement. Signed-off-by: ART3 <822f566773c6b5ec5de21614f3900e5dd78a36d772fb826a3ee0f4b140d68255@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested, narrowly: one P2 inline, plus one non-blocking P3.
Re-reviewed head 00cd0ba412730c2ac36403ff46a1ef0bc0a097c1 against base 5d2b08e2ff3bb40dc62f04015298ff319f4a4f8a.
The three earlier findings are addressed:
- Artifact conflicts. HTTP 409 and the socket
conflict:for kind 45010 now reach the outbox asfailed. I checked the premise against relaymain(95b018c87):/eventsreturns 409 only forIngestError::CanvasConflict, which canvas and artifact compare-and-set raise before mutation, and the socket forwards that text unchanged. - Message management and session-channel mode are wired through the real
ThreadPanelandMessageManagement, and the new tests assert both. - The single overlay state removes the panel-over-viewer case.
Merge criterion: the P2, with a regression that retargets an embedded thread within one channel.
Validation: source tracing at this head and the existing CI run, where all required checks and DCO pass. I reran no suites. conversation.ui.Thread still has no in-tree consumer, so the embedded surface remains runtime-unverified.
| return true; | ||
| }; | ||
| return ( | ||
| <MessageManagement session={session} channelId={channelId}> |
There was a problem hiding this comment.
[P2] Keep one channel visit across thread retargets
MessageManagement now sits inside the subtree keyed by messageId (line 42), so showing another thread of the same channel unmounts it and mounts a new one: leaveChannel, then enterChannel (MessageManagement.tsx:115–137). Re-entry deliberately reconciles the previous visit's message force (unread.ts:1045–1051; unread.test.ts:1204–1226 shows the manual intent returning to none).
Result: a reader marks a message unread in thread A, the page switches to thread B in the same channel, and the mark is gone, both the row state and the sidebar hint. Channels keeps it, because its owner wraps the whole channel workspace and a thread change does not end the visit (ChannelsPage.tsx:1279).
Scope the visit to session and channel rather than the thread key, and add a regression that changes messageId within one channel and asserts no leave/enter. Traced from source and the existing unread tests; not run through a page.
There was a problem hiding this comment.
🤖 ART3 (agent, via Bradley's account). Fixed in 3690b03. MessageManagement now wraps the keyed thread instead of sitting inside it, and is keyed by session, scope and channel; the thread below it is keyed by messageId. A retarget within one channel remounts only the thread and its overlay, so the visit is not left and re-entered.
Regression: keeps one channel visit while the thread is retargeted within a channel changes messageId and asserts enterChannel was still called once and leaveChannel not at all, then changes the channel as the barrier (leave c, enter d). It fails on 00cd0ba (enterChannel called twice) and passes now.
One consequence, the same as in Channels: a Delete confirmation that is open when the page retargets within the channel now stays open rather than closing with the thread.
| close={() => setOverlay(undefined)} | ||
| /> | ||
| )} | ||
| </MessageManagement> |
There was a problem hiding this comment.
[P3, non-blocking] Render the recovery status as well
Channels and Sessions pair this owner with <MessageManagementStatus /> (ChannelsPage.tsx:986, SessionsPage.tsx:221); the embedded thread does not.
The composer shows a failed edit and Retry while it stays open. Once it is cancelled, or the thread remounts with a failed or unknown edit still in the outbox, the message has no Edit (useMessageEdit.ts:30–36), a disabled Delete (MessageManagement.tsx:233–242) and no notice. Recovery is then only available by opening the channel in Channels. Rendering the status here covers it.
There was a problem hiding this comment.
🤖 ART3 (agent, via Bradley's account). Done in 3690b03: <MessageManagementStatus /> renders above the thread, inside the same channel-scoped owner. It is channel-wide, as in Channels, so it also lists a failed edit or deletion from another thread of that channel. Test: offers recovery for a failed edit left in the outbox (fails on 00cd0ba, passes now). The docs paragraph mentions the notice, since the page owns placement around it.
MessageManagement sat inside the subtree keyed by messageId, so showing another thread of the same channel ended the channel visit and started a new one. Re-entry reconciles the previous visit's message force, so a "Mark unread" made in the first thread was lost. Channels keeps it, because its owner wraps the whole channel. MessageManagement now wraps the keyed thread and is itself keyed by session and channel, so only the thread and its overlay remount on a retarget. Also render MessageManagementStatus there, as Channels and Sessions do. A failed or unconfirmed edit left in the outbox removes Edit and disables Delete on its message; without the notice an embedded thread offered no way to retry or discard it. Signed-off-by: ART3 <822f566773c6b5ec5de21614f3900e5dd78a36d772fb826a3ee0f4b140d68255@buzz.block.builderlab.xyz>
* origin/main: (27 commits) Let plugin pages publish NIP-AR artifacts and embed the host thread view (#434) test(app): migrate entity-navigation test off removed buzz://open locator API (#463) Show agent activity in navigation (#423) test(browser): hold motion when it commits, not on its start event (#459) fix(navigation): ignore unknown query parameters on Buzz links and remove the buzz://open locator (#457) feat(design-system): distinguish controls on floating surfaces (#429) feat(native): add community extras and media preparation (#450) Clone inventory identities through reviewed text and fresh identity creation (#289) feat(communities): add right-click actions to the community rail (#400) fix(messages): keep a send reveal pending until its scroll runs (#454) fix(messages): reserve a stable scrollbar gutter on the channel feed (#451) fix(sidebar): list plugin pages as sidebar rows via an opt-in primary flag (#401) feat(channels): surface canvas content in channel settings (#426) fix(profiles): remove redundant presence status row (#394) test(browser): count live retries once the page handles startup controls (#443) feat(composer): host-owned resource links for the Projects picker (#445) feat: support native read state and recent channel activity (#444) feat(native): serve relay media and uploads in packaged builds (#433) feat(channels): suggest joined channels in the composer (#446) feat: support native agent activity, library, memories, and community resolution (#441) ... Signed-off-by: Codex <noreply@openai.com>
…followup * origin/main: Group inventory by community and use compact rows outside the current community (#290) chore: enable Cmd+R reload in production builds (#468) ci: run playwright jobs in the pinned docker image (#469) fix(profile): let the web profiling page follow the browser window size (#467) Let plugin pages publish NIP-AR artifacts and embed the host thread view (#434) Signed-off-by: Tree Trunks <6ba22921d9dc2ad0aa6ecdf63787ddd24726e266d866da31af69f2e4e146ace5@buzz.block.builderlab.xyz>
Two host changes so a plugin page (e.g. a task board built on NIP-AR artifacts) can run as an ordinary external plugin without host-specific code.
What changes
1. Plugins can publish kind 45010 (NIP-AR artifacts)
nativeWriteKinds(src/features/relay/native.ts), to the Rustvalidate_eventallowlist (src-tauri/src/relay.rs), and to the web dev broker's write kinds and publish path./events, or a socketOK false "conflict: …"for 45010, reaches the outbox asfailedwith a fixedconflict:error, so the plugin can reload and reconcile instead of retrying a stale revision. The dev broker answers a proven conflict with 409, like the relay. Internal errors and unknown prefixes stayunknown.2.
conversation.ui.Threadconversation.ui.Thread({ session, scope, channelId, channelName, messageId })next to the existingui.Composerandui.Message.ThreadPanelwithout its side-panel header or close button, and Esc does not dismiss it.ThreadPanelis embedded whenevercloseis omitted.MessageManagementwith the channel id). The visit is scoped to session and channel, so it lasts across threads of the same channel; only the thread and its modal remount on a retarget.docs/plugin-architecture.mdas part of the existing host-matched conversation preview. It is not a stable SDK.Non-goals
Notes for review
src/features/conversation/service.tsxis markedFOUNDATION. The change adds only theui.Threadcomponent, which stays within that file's "shared conversation UI, never another data owner" boundary. baxen explicitly requested it.acceptPublishtreats any 409 as a rejection because it does not know the event kind. The relay's/eventsreturns 409 only forIngestError::CanvasConflict(canvas and artifact compare-and-set), which refuses before mutation.Validation (head
3690b037)tsc --noEmit, biome check on the changed filesvitest run: 456 files, 5591 tests passcargo test --lib relayinsrc-tauri(44 pass), last run at00cd0ba4; no Rust change sinceEmbeddedThread.test.tsx(realThreadPanel+MessageManagement; row, composer and viewer leaves reduced; includes a same-channel retarget that asserts no leave/enter of the channel visit, and the failed-edit notice),signed-admission.test.ts(signed HTTP transport + Outbox, 409 vs 500),relay-broker-live.test.mjs(broker + socket + Outbox,conflict:vs internal error)ui.Threadand 45010 writes against staging during the earlier prototype.🤖 Generated with Claude Code