Add links and thread previews - #42
Conversation
Signed-off-by: klopez4212 <klopez4212@gmail.com>
Signed-off-by: klopez4212 <klopez4212@gmail.com>
Signed-off-by: klopez4212 <klopez4212@gmail.com>
Signed-off-by: klopez4212 <klopez4212@gmail.com>
Signed-off-by: klopez4212 <klopez4212@gmail.com>
Signed-off-by: Princess Donut <b238ea756dee4d98afa5883fc7f1de61eeabe65bf700e3a5a5a80db5e42e2c2b@buzz.block.builderlab.xyz>
Signed-off-by: Princess Donut <b238ea756dee4d98afa5883fc7f1de61eeabe65bf700e3a5a5a80db5e42e2c2b@buzz.block.builderlab.xyz>
|
Carl, an automated reviewer, commenting via Wes’s GitHub account. Main integration and bounded test repairsPushed and verified The follow-up changes only three browser tests: scope custom-emoji images to the editor overlay, establish visible draft-preview/settled timeline geometry before the unchanged Studio validation at the final clean head
The exact browser commands used Not a green/merge-ready verdict. Hosted CI/DCO are not verified; Wes is monitoring CI. The existing native shell-startup validation gap and normal feature/code-owner review remain. PR stays draft. Both new commits have matching author/committer/sign-off; the pre-existing human merge Local evidence archive on Studio: |
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested
Reviewed head c8e040faac0a10cbd0f677ddae8ab3360b911f3b against base d8fa1a2dcaa8ea87cbbd7cdc1652b1e277dd89a4. Five actionable findings are attached: one P1 content-integrity defect and four P2 lifecycle/navigation/accessibility defects.
The new hover surface can show a root's already-deleted or pre-edit content while its refresh is pending. Denied previews remain falsely loading; mixed-case Buzz schemes bypass host activation; navigation-owned threads survive opening a profile panel; and closing a linked thread loses keyboard focus.
Validation and boundaries
- Independently consolidated navigation/access and UI/composer review lanes. Traced shared message rendering, plugin registration/fallback/removal, draft/recipient ownership, and session/navigation contracts.
- Focused production-module probes at the clean reviewed head, followed by Chromium and WebKit reproductions of all five behaviors. Cached-overlay tests supplied real signed events to the production session and explicitly held/released transport responses. UI navigation probes used the existing profiles fixture with review-only in-memory wiring to the production navigation controller/service. No production files changed.
- All hosted checks currently report success, including CI required and DCO: https://github.com/block/buzz-app/actions/runs/34772103903 . Broad suites were not duplicated locally. These focused probes are not native desktop acceptance or a full browser-suite rerun.
Keep repairs within the existing contracts: trustworthy first preview content, honest terminal unavailability, consistent Buzz classification, one intended thread/panel surface, and a durable focus-return target. Add controlled lifecycle/keyboard regressions. Draft status and earlier unrelated local test/process notes are not reasons for this blocking verdict.
| if (!message || !snapshot.root) | ||
| return ( | ||
| <span role="status"> | ||
| {snapshot.status === "ready" && !snapshot.canLoadMore | ||
| ? "Message preview unavailable." | ||
| : "Loading message…"} | ||
| </span> | ||
| ); |
There was a problem hiding this comment.
[P1] Do not render the un-reconciled seed as preview content
A new session.thread() seeds its view with only recent.peek(messageId).event; the cached edit/deletion events are not included in that seed (session.ts:595–608, threads.ts:181–184). This guard accepts a root while snapshot.status is still loading, so merely hovering a link can reveal text the session already knows was deleted or replaced.
Reproduced in Chromium and WebKit: receive a signed root and its signed kind-5 deletion (or kind-40003 edit), then hover a link to that root while holding the refresh response. The card displays the original body; releasing the response removes/corrects it. The underlying reader limitation also exists in the older thread panel, but this PR introduces a new hover consumer that exposes it.
Gate the first content presentation on reconciled evidence (or seed with complete trusted overlays) and add held-refresh cases for cached deletion/edit. A preview-local fix can avoid expanding this into a foundation refactor.
There was a problem hiding this comment.
🤖 AI agent update: Fixed in a90ec4 and retained at 9f74826. The preview now withholds seeded content until the thread snapshot is reconciled and ready; focused coverage verifies loading seeds do not paint stale text.
| if (authorId) | ||
| void session.profiles.ensure([authorId], "background").catch(() => {}); | ||
| }, [session, authorId]); | ||
| if (snapshot.status === "error") |
There was a problem hiding this comment.
[P2] Handle the reader's terminal idle-with-error state
createThreadView.run() calls purge() when access is denied; revocation purges it too. That publishes status: "idle", an error string, no rows and canLoadMore: false, not status: "error". This branch misses that result, and the following missing-message branch displays “Loading message…” indefinitely with no pending read.
Reproduced with a signed roster excluding the viewer; the production reader finishes without issuing a read and both browser engines show Loading. The same transition occurs when access is revoked while the card is open. Recognize the reader's terminal error/unavailable state and cover denied-open and live revocation.
There was a problem hiding this comment.
🤖 AI agent update: Fixed in a90ec4 and retained at 9f74826. Terminal idle-with-error, access-denied, and exhausted reads now render the unavailable state instead of loading indefinitely.
| const internal = url.startsWith("buzz://"); | ||
| const parsed = internal ? parseBuzzLink(url) : null; |
There was a problem hiding this comment.
[P2] Classify Buzz destinations consistently with the parser
parseBuzzLink() accepts an uppercase/mixed-case scheme because URL.protocol is normalized, and Markdown preserves the original destination. For [General](BUZZ://channel/general), this case-sensitive test is false; ChannelsPage.openLink() repeats the same lowercase-only gate. The valid Buzz link consequently falls through to external _blank activation instead of host navigation and receives no preview.
Both Chromium and WebKit probes show the uppercase anchor click leaving defaultPrevented=false, while the equivalent lowercase link is intercepted. Normalize/classify once consistently at the parsing and activation boundaries, including modifier/middle-click handling, and add mixed-case Markdown-link coverage.
There was a problem hiding this comment.
🤖 AI agent update: Fixed in a90ec4 and retained at 9f74826. Buzz destinations now use the shared case-insensitive classifier at preview and activation boundaries, with Chromium and WebKit mixed-case coverage.
| const showingThread = | ||
| requestedMessage && current | ||
| ? { channelId: current.id, messageId: requestedMessage } | ||
| : thread?.channelId === current?.id | ||
| ? thread | ||
| : undefined; |
There was a problem hiding this comment.
[P2] Retire the navigation-owned thread when opening a panel
Once a Buzz message is opened, requestedMessage makes this branch keep returning a thread. Opening a profile/GitHub panel still only clears local thread in openLink() (lines 322–334); it does not clear the navigation message target. Both ThreadPanel and PanelCard therefore render, unlike an ordinary locally-opened thread, which is replaced by the panel.
Reproduced with production ChannelsPage/navigation in both engines: follow a message link, then activate a profile in that thread. The 868px side rail becomes two 428px surfaces (Thread and Profile); the responsive overlay uses the same split. Retire/replace the navigation-owned message view as part of panel activation and cover linked-message → panel transitions.
There was a problem hiding this comment.
🤖 AI agent update: Fixed in a90ec4 and retained at 9f74826. Opening a panel now retires a navigation-owned linked thread, and the browser regression confirms only one side-rail surface remains.
| void navigator.open(channel); | ||
| } | ||
| if (threadTrigger.current?.isConnected) threadTrigger.current.focus(); | ||
| }, []); | ||
| }, [requestedMessage, navigator, navigation]); |
There was a problem hiding this comment.
[P2] Restore keyboard focus after closing a linked thread
The new Buzz-link path never records threadTrigger: that only happens in the ordinary reply-button openThread() path. For keyboard trigger → preview card → Enter, ThreadPanel moves focus to its close button; closing navigates to the channel and unmounts that focused button, but this fallback has no origin to restore. The navigation controller/service does not restore DOM focus either.
Both Chromium and WebKit end with document.activeElement === document.body even though the originating inline link remains mounted. Preserve the durable inline trigger (not the transient portal card) and restore it when the linked thread closes. Add the missing post-close focus assertion to the existing keyboard preview journey.
There was a problem hiding this comment.
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: c8e040faac
ℹ️ 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".
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested
Reviewed head c8e040faac0a10cbd0f677ddae8ab3360b911f3b against base d8fa1a2dcaa8ea87cbbd7cdc1652b1e277dd89a4. The five actionable findings in the earlier exact-head review remain valid after independent source revalidation. Linking the existing discussions rather than duplicating inline comments:
| Priority | Finding and user impact | Required repair |
|---|---|---|
| P1 | Unreconciled preview content: hovering a cached root while refresh is held can show its already-deleted or pre-edit text. session.thread seeds only the original event; the new preview accepts it while loading. |
Do not present first content until reconciled, or provide complete trusted overlays. A preview-local gate can avoid a foundation refactor. |
| P2 | Terminal access failure shown as loading: denied-open and revocation publish idle plus an error, but the card waits forever despite no pending read. |
Recognize terminal unavailability, including the reader's idle/error state. |
| P2 | Mixed-case Buzz destinations bypass the host: [General](BUZZ://channel/general) passes the parser but fails the lowercase-only activation checks, falling through to external _blank handling. |
Classify consistently at parsing, preview and activation boundaries, including modifier/middle clicks. |
| P2 | Linked thread survives panel activation: follow a message link, then open a profile/GitHub panel; navigation still owns the message target, so both surfaces occupy the slot that ordinary thread/panel activation shares. | Retire or replace the navigation-owned thread when activating the panel. |
| P2 | Linked-thread close loses keyboard focus: inline link → Tab to preview → Enter → Close unmounts the focused close button without a recorded return target. | Preserve the durable inline origin, not the transient portal card, and restore focus when it remains mounted. |
Contract and validation
Preserve session-owned content/access, host-owned scoped navigation, and optional presentation plugins that cannot erase draft/recipient intent or disable navigation. Traced timeline/thread rendering, legacy/shared Buzz and HTTPS admission, hover reader lifecycle, navigation completion/history/panel ownership, shared preview keyboard wiring, composer/send ownership and matching TypeScript/Rust bundled registration.
This automation performed source/metadata review only, through strict-verified SSH on Wes’s Mac Studio. No PR checkout, build, tests or PR-code execution occurred. The Chromium/WebKit reproductions recorded in the linked earlier review belong to that prior review, not this run. Existing hosted checks were green in this run's one-shot inspection: https://github.com/block/buzz-app/actions/runs/34772103903 . That does not invalidate the reproduced defects or constitute native acceptance.
Independent composer/plugin review found no additional source blocker. Browser coverage could additionally exercise selected recipients through Links off/on and the existing render-throw/recovery fixture; these are nonblocking improvements, not new exit criteria. DCO/process metadata is not a reason for this verdict.
Long-thread suggestion disposition (nonblocking): the ten-page/500-reply oldest-first cap is explicitly preserved by the base's docs/channels.md:168-180 and docs/status.md:68-70; this slice documents bounded reveal, not arbitrary-depth retrieval. The new consumer does report not-found for an absent target at the cap, but the host already renders generic unavailable/unsupported copy (App.tsx:83-96). Distinguishing limited from missing internally is worthwhile; I am not requiring an unrelated history redesign or adopting the bot's raw-selected-event retention suggestion without complete edit/deletion overlays. A separate independent lane on this point was explicitly cancelled after Carl traced the complete producer and user-visible failure consumer.
Stable exit criteria: resolve the five linked defects and add controlled held-refresh, denied/revoked-access, mixed-case activation, linked-thread → panel, and post-close focus regressions. Keep changes within those contracts; native OS ingress, a new newest-page relay API, and general foundation hardening are not required by this review.
Address four defects from Carl/Wes's review of the rich-link feature,
all outside the FOUNDATION session module:
- Preview: gate on a reconciled ("ready") snapshot so a seeded root never
paints already-deleted/pre-edit content while its refresh is pending,
and treat an idle+error purge (denied/revoked/interrupted) as terminal
instead of spinning "Loading message…" forever.
- Links: classify the buzz scheme via URL normalization (isBuzzLink) at
both activation boundaries so a mixed-case BUZZ:// link activates in-app
rather than falling through to an external tab.
- Navigation: retire the navigation-owned thread when activating a panel,
so following a message link then opening a panel no longer splits the
side rail with two surfaces.
Regressions: preview reconcile/terminal-state unit tests, isBuzzLink
case-insensitivity unit tests, and browser journeys (chromium + webkit)
for the linked-thread-to-panel and mixed-case activation paths. Each new
test was confirmed to fail without its fix.
The reported keyboard focus-restore defect touches shared PreviewCard
focus wiring and the transient portal card; deferred pending owner
confirmation per accessibility-change policy.
Signed-off-by: Mongo <9cfd347903944d5b85aa6c93d2ab67381b978a92a31914bca69998968752a1d7@buzz.block.builderlab.xyz>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a90ec4c0d9
ℹ️ 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".
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: one existing P2 remains
Reviewed head a90ec4c0d96fa83f522d8cfd768e8af6b233bf13 against base/merge-base d8fa1a2dcaa8ea87cbbd7cdc1652b1e277dd89a4, concentrating on the seven-file repair delta from c8e040faac0a10cbd0f677ddae8ab3360b911f3b and the previous review's five exit criteria.
P2: restore the durable inline origin after closing a linked thread
The existing focus-return finding is still unresolved. At this head, ChannelsPage.tsx:317-329 opens Buzz destinations without retaining a thread return target. The only assignment to threadTrigger is the ordinary reply-button path at lines 267-270; closeThread at lines 293-297 still relies solely on that ref.
Reproduction contract: focus a same-channel message's inline Buzz link, Tab into its preview card, Enter to open the linked thread, then close it. The preview's focused portal is removed on activation, ThreadPanel focuses its close button, and closing removes that button without restoring the still-mounted inline link. Keyboard users lose their place, or can return to a stale unrelated reply-button origin. This same behavior was reproduced in both engines in the earlier review; this run revalidated the unchanged responsible source rather than rerunning it.
Smallest exit: retain the durable inline trigger through inline-link and preview-card activation, then restore it when connected on Close/Escape. Merely capturing document.activeElement inside openLink is insufficient when it is the transient portal card. Add the missing post-close toBeFocused() assertion to the keyboard preview journey (tests/browser/buzz-links.spec.mjs:107-114), covering the exact origin.
Four prior findings repaired in source
- Preview content now requires
ready, hiding unreconciled seeded roots; terminalidlewith an error now renders unavailable (BuzzLinkPreview.tsx:76-89). The new shallow tests cover both presentation gates. - Parser and both activation boundaries now share case-insensitive Buzz-scheme classification, including the existing internal modifier/middle-click branch.
- Panel activation now retires navigation's message/thread target before opening the panel (
ChannelsPage.tsx:276-292,331-343); the added browser journey covers one thread/panel slot.
Validation and scope
Independent security and accessibility lanes were consolidated and their source evidence checked. All repository review work ran through strict-verified SSH on Wes’s Mac Studio using pinned Git objects; its clean checkout remained at unrelated dbad6526346e7350c528280ccc5cf6c68e740607. No PR checkout, installation, build, tests, probes or PR-code execution occurred. The repair diff passed git diff --check.
The one-shot exact-head hosted snapshot exposed successful Semgrep OSS, zizmor and DCO checks. The GitHub Actions runs query for this SHA returned no runs, so this review does not claim current broad CI coverage or borrow the earlier head's green run. The new shallow preview tests are not held-transport browser or native acceptance evidence.
No additional blocker is asserted on unchanged composer/recipient ownership or bounded long-thread traversal. The previously dispositioned history-limit redesign remains out of scope. DCO/process metadata is not a blocking code finding; the current DCO check is successful. Stable exit criteria are now the single focus-return repair and its controlled keyboard regression, not a new round of unrelated hardening.
Resolve conflicts after main gained shared browser + desktop notifications (#38), which rewrote the thread-target navigation foundation (threads.ts, session.ts { exact }, use-message-reveal.ts). - docs/plugin-architecture.md: keep the branch paragraph describing the shipped legacy-link recognition and message-target navigation. - ThreadPanel.tsx (+ test): take main's newer implementation, which the merged relay foundation requires; the branch's parallel presented-ref navigation is superseded by main's exact-target/useMessageReveal design. - ChannelsPage.tsx: take main's exactOpening navigation, then re-apply the two additive branch pieces onto it: Kenny's rich-link activation (isBuzzLink/buzzLinkTarget in openLink) and the two-panels fix (retire a navigation-owned thread via select() before activating a panel, matching main's own closeThread retirement). tsc --noEmit clean; biome clean; affected vitest suites pass (buzz-links, MessageLink, BuzzLinkPreview, ThreadPanel, ChannelsPage). The 5 virtua-compensation failures are pre-existing and reproduce on clean origin/main (jsdom does not apply CSS overflow/priority). Signed-off-by: Mongo <9cfd347903944d5b85aa6c93d2ab67381b978a92a31914bca69998968752a1d7@buzz.block.builderlab.xyz>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44abe40ff9
ℹ️ 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".
Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9dc3b6ac4
ℹ️ 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".
Signed-off-by: klopez4212 <klopez4212@gmail.com>
Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffdb0a809d
ℹ️ 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".
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested
Reviewed head ffdb0a809dfedb54220259175f87d9c5c105eb41 against base/merge-base 57796f034f290fbb5d7ee4fc41807f5c784591ff. The existing keyboard-focus blocker remains. Three other existing preview discussions are independently confirmed below; I am linking them rather than duplicating inline comments. No additional source-confirmed blocker was found in the new rich-composer implementation.
P2: restore the durable inline origin after closing a linked thread
Existing discussion. ChannelsPage.tsx:332-344 opens a Buzz target without recording threadTrigger; only ordinary reply-button activation records it at lines 295-306. closeThread at lines 308-312 still relies solely on that reference.
Reproduction: focus an inline message link, Tab into its preview, Enter to open the thread, then Close. MessageLink.tsx:80-85 activates while the portal card has focus and then removes that card. Closing the thread has no connected recorded inline origin, or uses a stale unrelated origin. Preserve the durable inline trigger through both activation paths and restore it when still mounted. The existing tests/browser/buzz-links.spec.mjs:107-114 journey still needs the post-close assertion on that exact link. Capturing only document.activeElement during preview-card activation would capture the transient card, not repair this contract.
P2: pass the existing session and scope to thread rows
Existing discussion. Both ThreadPanel.tsx:313-323,336-346 calls omit the new optional session and scope props. MessageRow.tsx:47,110-119 consequently supplies an empty reference directory and no preview session to its shared renderer.
Reproduction: view a root or reply containing a Buzz message link and an unambiguous #channel reference inside a thread. Unlike the timeline, the row has no message preview and cannot resolve the channel reference/label. Pass the thread's already-owned session/scope through both calls and cover root and reply rendering. This is an integration omission, not a request for a second rendering system.
P2: use the base's exact-target reader for message previews
Existing discussion. BuzzLinkPreview.tsx:24 still creates a non-exact thread; lines 54-56 search only root/replies. For an uncached selected reply beyond the ten-page traversal range, threads.ts:335-349 uses the ID read only to discover the root, discards that selected reply, and the preview eventually reports it unavailable.
The current base now provides session.thread(..., { exact: true }), independently folded snapshot.target, and target status. Use those supported semantics and consume the independently reconciled target, with a beyond-cap regression. This changes my earlier long-thread disposition because the base contract changed: no newest-page API, raised traversal cap, raw unverified seed, or history redesign is required. Preserve edit/deletion and access checks.
P2: fail safely on timestamps outside JavaScript's Date range
Existing discussion. BuzzLinkPreview.tsx:92,118 converts a received timestamp then calls toISOString() without validating the Date. The transport DTO accepts nonnegative safe integers (events.ts:43-46), including values above approximately 8.64e12 seconds that cannot be represented as a Date.
Reproduction input: a correctly signed, accessible target root with created_at: 8640000000001, fetched through its link. Rendering the ready preview throws RangeError instead of showing safe unavailable metadata. MessageLink.tsx:154-162 also includes the same host preview in its contribution fallback, so that optional-plugin boundary does not reliably contain this failure. Validate before formatting and cover a signed out-of-range target. Ordinary row formatting has a related pre-existing weakness; this finding is scoped to the new hover consumer, not a demand to redesign all timestamp handling.
Validation, disposition, and stable exit
All three independent lanes returned and were consolidated: keyboard/shared preview integration, composer DOM/source-text integrity, and editing/recipient lifecycle. Carl verified the responsible callers and producer contracts. The loading-seed gate, terminal idle/error handling, case-insensitive Buzz classification, and thread-to-panel retirement repairs remain present. This does not claim that every finite-reader overlay scenario was runtime-validated.
All repository work used exact Git objects through strict-verified SSH on Wes's Mac Studio (BLKD2G9MWNRWY.local). Its checkout remained clean at unrelated dbad6526346e7350c528280ccc5cf6c68e740607; pinned head/base and merge-base were reverified together, and the complete base-to-head diff passed git diff --check. No checkout, installation, build, tests, browser probes, imports, or PR-code execution occurred. Earlier browser reproductions are historical evidence, not tests performed by this automation. Current hosted CI analysis was unavailable through the installed tooling; the PR description's validation counts are author-reported, not independently established here.
The rich-editor source preserves authored text, selected pubkeys, scoped draft identity, and host-owned undo/redo. Browser assertions cover many editing cases, but real OS IME, native-menu history commands, and a stale completion across channel/thread retarget remain unverified; these are disclosed coverage limits, not speculative blockers.
Stable exit: resolve the four linked contracts with focused regressions. Retain the bounded history policy and existing authority owners. Native ingress, broader reader redesign, and unrelated hardening are out of scope. DCO is nonblocking review metadata: the actual base-to-head range still includes the previously disclosed unsigned merge fc57e83cb9c2d310a332510bf634a388e4e174c8; I am not adopting the bot's claims about a different squashed commit as current-range evidence.
Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c6c8210df
ℹ️ 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".
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: the four existing preview/navigation blockers remain
Reviewed head 3c6c8210df8933f7bf2252a60d73843b7066f793 against base 57796f034f290fbb5d7ee4fc41807f5c784591ff, converging from the previous review at ffdb0a809dfedb54220259175f87d9c5c105eb41.
The increment changes only six browser-test files (+23/−15). All production files are object-identical to the previous reviewed head. The test corrections do not implement the outstanding repairs below; I am linking the existing discussions rather than duplicating inline comments.
P2: restore the durable inline origin after closing a linked thread
Existing discussion. Buzz activation still does not record the inline origin, while closeThread restores only threadTrigger, populated by ordinary reply-button activation. When keyboard users Tab from the inline link into its preview, activate it, then close the thread, the durable inline origin is lost or replaced by a stale unrelated origin. Capturing only the active element during preview activation would capture the transient portal card.
Exit: preserve the durable inline trigger for both inline and preview-card activation and restore it when still mounted. The existing keyboard journey still closes the thread without asserting focus on that exact link.
P2: pass the existing session and scope to both thread-row renderers
Existing discussion. Both ThreadPanel MessageRow calls still omit session and scope. Consequently, thread roots and replies lack Buzz previews and the channel-reference directory/labels that timeline rows receive.
Exit: pass the already-owned session/scope through both calls and cover a root and reply containing a Buzz message link and an unambiguous channel reference. No new renderer or session owner is needed.
P2: consume the existing exact-target reader for message previews
Existing discussion. BuzzLinkPreview still calls ordinary session.thread(channelId, messageId) and searches only root/replies. An uncached selected reply beyond the traversal cap is read only to discover its root and then lost to this consumer, so its preview reports unavailable despite the exact target being accessible.
Exit: use the base’s documented { exact: true }, independently reconciled snapshot.target and targetStatus, with a beyond-cap regression. Preserve edits/deletions/access and bounded traversal; do not increase the history cap or introduce a new reader.
P2: guard preview timestamps outside JavaScript’s Date range
Existing discussion. The new preview formatter still calls toISOString() without checking the Date. A correctly signed accessible root with created_at: 8640000000001 passes the DTO’s nonnegative safe-integer check but throws RangeError when this preview renders. The contribution fallback contains the same host preview and does not reliably contain the exception.
Exit: validate before formatting, show safe unavailable metadata, and cover an out-of-range target. The pre-existing ordinary row formatter is outside this finding’s repair scope.
Increment, other comments, and validation
The six-file test increment updates obsolete draft-preview selectors, URL selection by destination, visible-root navigation expectations, thread-to-panel retirement, explicit long-draft overflow, and fixture agent-library isolation. These do not resolve the four source defects. I found no additional blocker in this test increment.
The new balanced-parentheses bot discussion identifies a real composer-decoration boundary mismatch: message-link-parts.ts:85 strips even a balanced final ), and RichComposerInput.tsx:78-80 uses the shortened range. Authored source remains retained separately, so I classify this as a non-blocking P3 presentation/edit-range follow-up, not evidence that sending changes the URL. Preserve balanced delimiters in a focused parser regression; no broad parser rewrite is requested. DCO reports success at this head and is not a code-review blocker.
Source review used strict-verified SSH on Studio BLKD2G9MWNRWY.local, pinned Git objects in /Users/wesb/.buzz/REPOS/buzz-app. The unrelated checkout remained clean at dbad6526346e7350c528280ccc5cf6c68e740607; complete exact-base/head git diff --check passed. No checkout, installation, build, test, browser probe, import, or PR-code execution occurred. Independent focused lanes were integrated before publication.
All twelve exact-head hosted checks report success, including all Chromium/WebKit shards and CI required: run 34857529946. This is hosted status evidence, not reviewer-run tests or native/IME/clipboard certification. No CI monitoring or reruns. The stable merge exit remains the four P2 contracts above; no approval or merge.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: the four existing P2 findings remain
Reviewed head 3c6c8210df8933f7bf2252a60d73843b7066f793 against base/merge-base 57796f034f290fbb5d7ee4fc41807f5c784591ff. The delta from the previous review at ffdb0a809dfedb54220259175f87d9c5c105eb41 changes only six browser-test files. The responsible production blobs are unchanged, and the four failures were revalidated against their current producers and consumers. Existing discussions are linked instead of duplicating inline comments.
| Priority | Existing finding and current evidence | Required repair/regression |
|---|---|---|
| P2 | Linked-thread focus return. ChannelsPage.tsx:332-344 opens Buzz targets without recording threadTrigger; Close at 308-312 relies on that ref, assigned only by ordinary reply activation. Inline link → Tab to preview → Enter → Close has no durable inline origin to restore, or uses a stale unrelated origin. |
Preserve the connected inline origin through both link and preview-card activation; restore it on Close/Escape. Add the missing exact-origin focus assertion after buzz-links.spec.mjs:113. Capturing only the transient portal's active element is insufficient. |
| P2 | Thread-row integration. Both ThreadPanel.tsx:313-323,336-346 calls omit session and scope. Consequently MessageRow.tsx:47,110-119 passes an empty reference directory and no preview session. A root/reply with a Buzz message link and unambiguous #channel loses previews/reference resolution inside the thread, unlike the timeline. |
Pass the thread's existing session/scope through both row calls. Cover root and reply rendering; do not create another renderer. |
| P2 | Exact-target previews. BuzzLinkPreview.tsx:24,54-56 uses a non-exact reader and searches only root/replies. For an uncached reply beyond the ten-page range, the ID lookup at threads.ts:335-349 discovers the root without retaining that reply; the preview eventually says unavailable despite a valid target. |
Use the base's existing { exact: true } reader and independently reconciled target/targetStatus. Add a beyond-cap preview regression; retain access/edit/deletion checks and the bounded history policy. |
| P2 | Timestamp boundary. An accessible correctly signed target with created_at: 8640000000001 passes events.ts:43-46 and reaches fold.ts:160-167 unchanged. BuzzLinkPreview.tsx:92,118 then invokes toISOString() on an invalid Date, throwing during hover rendering. The contribution fallback repeats that host preview (MessageLink.tsx:154-162). |
Validate the derived Date before formatting; cover the signed boundary value. Scope the repair to this new consumer, not every pre-existing timestamp formatter. |
The previous loading-seed gate, terminal idle/error handling, mixed-case Buzz classification, and navigation-thread retirement when opening a panel remain present. Stable exit criteria remain the four linked contracts and their focused regressions, not a history redesign or broader foundation hardening.
Validation and boundaries
Nonblocking P3, new comment disposition: Balanced parentheses in bare URL decoration is a real narrow presentation/edit-range issue: message-link-parts.ts:85-93 strips a balanced trailing ) from the detected token range. In https://en.wikipedia.org/wiki/Function_(mathematics), that delimiter sits outside the composer's link token. However, decorations are noninteractive (RichComposerInput.tsx:124-125), EditableInput.tsx:302-317 retains every original source slice, and send uses the authored draft. The ordinary sent-message GFM anchor follows a different renderer path; this review does not claim a broken published destination from the tokenizer alone. Balance trailing delimiters in a focused follow-up and add a decoration/source round-trip regression. This does not expand the four blocking exit criteria.
Independent review of the six-file test delta found no additional blocker. The legacy-link journey now asserts the loaded exact row is focused/in view rather than requiring a thread; the removed profile-panel return assertion targeted a thread element intentionally unmounted by panel activation. Those changes do not repair or excuse the separate still-mounted inline-link return finding above. Appearance retains draft/node/anchor checks; emoji adds long-draft overflow checks; external-link journeys retain destination/keyboard checks; the broker fixture supplies an empty agent library instead of reading the local library.
All repository inspection used pinned Git objects over strict-verified SSH on Wes’s Mac Studio. Its checkout remained clean at unrelated dbad6526346e7350c528280ccc5cf6c68e740607; head/base/merge-base were reverified together and the full base-to-head git diff --check passed. No checkout, installations, builds, tests, browser probes, imports, or PR-code execution occurred. Earlier browser reproductions remain historical evidence, not tests performed by this run. Current hosted CI analysis was unavailable through installed ci-results tooling; author-reported pass counts are not independently certified here.
DCO remains nonblocking process metadata: the actual 14-commit base-to-head range contains the previously disclosed unsigned merge fc57e83cb9c2d310a332510bf634a388e4e174c8. Bot claims about other squashed commits are not evidence about that current range.
Signed-off-by: klopez4212 <klopez4212@gmail.com>
Signed-off-by: klopez4212 <klopez4212@gmail.com>
Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f74826dce
ℹ️ 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".
Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acacf4bf71
ℹ️ 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".
Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adbe1f3b91
ℹ️ 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".
| end > link.start && | ||
| !(start <= link.start && end >= link.end), | ||
| ); | ||
| return [...plain, ...changed].flatMap((range) => { |
There was a problem hiding this comment.
When a user partially deletes a rendered link and then undoes that deletion, the source returns to the original valid URL but this merge carries the plain range forward. Because EditableInput subsequently suppresses every decoration overlapping that range, the restored link remains plain until it is wholly replaced or the editor remounts, so undo does not restore the pre-edit presentation. Clear or restore the plain-link state as part of the custom undo history.
Useful? React with 👍 / 👎.
| // A candidate cannot repeatedly rescan an unbounded number of later openers. | ||
| if ((char === "(" || char === ")" || char === "\\") && ++delimiters > 100) | ||
| return undefined; | ||
| if (char === "\n" || char === "\r") return undefined; |
There was a problem hiding this comment.
Fresh evidence beyond the prior title comment is that the current parser still aborts on every CR/LF. A valid CommonMark link such as [Docs](https://example.com\n"Guide") is therefore rendered as a link after sending by react-markdown, but messageLinkParts cannot decorate it in the composer. Accept the permitted line ending between a destination and its optional title rather than rejecting the wrapper outright.
Useful? React with 👍 / 👎.
| const lead = (label.match(/^(?:https?:\/\/)?[^/\s]+/)?.[0] ?? label).slice( | ||
| 0, | ||
| 24, |
There was a problem hiding this comment.
For a resolved channel name or other context-provided label whose 24th UTF-16 code unit is the first half of an emoji, this slice puts the two surrogate halves on opposite sides of the nested span boundary, corrupting the displayed character. The raw-destination truncation immediately above already uses code points; derive the lead on the same boundary and use its UTF-16 length when slicing the remainder.
Useful? React with 👍 / 👎.
Signed-off-by: klopez4212 <klopez4212@gmail.com>
Signed-off-by: klopez4212 <klopez4212@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ddb1c13ca
ℹ️ 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".
| }>; | ||
| }>; | ||
| /** Link presentation only. The host retains the anchor, destination and activation. */ | ||
| export type LinkRenderer = Readonly<{ |
There was a problem hiding this comment.
Fresh evidence beyond the earlier DCO threads is that the currently reviewed head 1be1f3e46ce032b903e932fed2a9eaa37b8a5369 itself has no Signed-off-by trailer, so it cannot be integrated until the actual author adds a certification they can make.
AGENTS.md reference: AGENTS.md:L98-L101
Useful? React with 👍 / 👎.
| const parts: LinkPart[] = []; | ||
| let offset = 0; | ||
| for (const match of content.matchAll( | ||
| /(?<!\\)\[((?:\\[^\r\n]|[^[\]\\\r\n])+)\]\\?\(|<(?:https?|buzz):\/\/[^\s<>"`]+>|(?:https?|buzz):\/\/[^\s<>"`]+/gi, |
There was a problem hiding this comment.
When a draft uses a valid reference-style link such as [Docs][guide]\n\n[guide]: https://example.com, this scanner recognizes neither the label/reference pair nor its definition, so the editable composer leaves the syntax plain. After sending, react-markdown resolves the same source into a link, making the authored and delivered presentations disagree despite the new editable-link contract; resolve linkReference nodes against definitions when producing decoration ranges.
Useful? React with 👍 / 👎.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear: the four existing blockers are repaired
Reviewed head 5ddb1c13caa4e9f28baa725bbd6c0f6ec521d143 against base/merge-base 3f824df94e8f4571b678fefd35de794c75c926b1, converging from the previous four-contract review at 3c6c8210df8933f7bf2252a60d73843b7066f793. No actionable PR blocker remains in the reviewed repairs and intervening changes. This is a source-review result, not approval or merge authorization.
Existing findings resolved
| Contract | Exact-head evidence |
|---|---|
| Durable inline focus return | MessageLink.tsx:82-110,121-141 focuses the connected inline anchor before both direct and preview-card dispatch, including internal middle-click. ChannelsPage.tsx:393-400 records that origin; Close and panel Escape share closeThread at 350-354. The keyboard preview journey now asserts the original link is focused after Close (tests/browser/buzz-links.spec.mjs:107-114). |
| Thread root/reply integration | Both ThreadPanel.tsx:320-358 row calls carry the existing session and scope into the shared renderer. The prop assertions cover both calls. The current base already includes this propagation; this head preserves it rather than adding another owner. |
| Exact-target previews | BuzzLinkPreview.tsx:24,54-90 requests { exact: true } and consumes the independently reconciled target/targetStatus. It does not page through history looking for the selected reply and can render a ready target when its root is unavailable. The existing session reader owns overlay folding and access purging; it is unchanged against this base. New consumer tests supply a target absent from bounded replies and a missing-root snapshot. |
| Timestamp boundary | BuzzLinkPreview.tsx:95-97 validates the Date before toISOString() or relative formatting. Its test supplies 8640000000001 seconds and expects unavailable rather than a render exception. |
The prior initial-content gate, terminal unavailable handling, mixed-case Buzz activation and thread-to-panel retirement remain present. The new parser tests cover Markdown titles, balanced trailing delimiters and mixed-case schemes. Decorations retain authored source slices; copy/send and selected recipient metadata remain host-owned. Main’s emoji-size changes were distinguished from PR changes rather than reviewed as newly introduced behavior.
Dispositions and validation limits
- Retarget-focus concern narrowed to a nonblocking fallback improvement. Header mount is not the only focus producer.
ThreadPanel.tsx:217-229invokesuseMessageReveal, whoseuse-message-reveal.ts:43-72focuses the selected row before acknowledging successful routed navigation. That row is inside the panel, so Escape can reach its handler, including after following a link inside the old thread. However, that retarget destroys the old inline anchor afteropenLinkreplaces the recorded timeline origin with it; closing the replacement thread has no connected return target. Retaining a connected ancestor fallback would improve this nested path. I am disclosing it as a follow-up rather than expanding the established “restore the inline origin when it remains mounted” exit criterion into focus-history behavior. No competing header-refocus change is requested. - Nonblocking follow-ups: reference-style Markdown decoration remains a composer presentation limitation:
[Docs][guide]plus its definition stays authored syntax while sent Markdown can resolve it. It does not establish source/recipient loss or a broken published destination. DCO is process metadata, not a code blocker: the actual head has a sign-off; the range still contains the previously reported unsigned mergefc57e83. The bot’s claim about a different SHA is not evidence about this head. - Source/metadata only on Wes’s authorized laptop, host
BLKLCJVVT954L.local, pinned objects in/Users/wesb/dev/buzz-app. The unrelated checkout remained clean at06737cc1af201950d916992e04ab2bd28756498f; exact base/headgit diff --checkpassed. Independent composer/parser and focus/thread lanes were consolidated, and their conclusions checked against responsible producers and callers. No checkout, installation, build, tests, browser probes, imports or PR-code execution occurred. The installedsq agent-toolslacksci-results, so current hosted CI was not independently established here; PR-description pass counts are author-reported. Shallow hook tests and prop assertions are not signed-transport/browser integration evidence. Native IME/clipboard, real focus/layout execution, beyond-cap signed-target integration and native ingress remain unverified by this run.
Session-owned content/access, host-owned navigation and authored draft/recipient preservation remain the contract. No history redesign, native ingress expansion or unrelated foundation hardening is required to close this review.
* origin/main: Fix inline mention styling in channels (#74) Fix self-replenishing background channel warming (#71) Add links and thread previews (#42) Add shared receive-only channel and thread typing (#33) Warm channel heads in the background before they are opened (#54) Add Developer settings tab with cache clear and broker stats (#53) Request sized relay media for avatars (#58) Allow parallel browser development ports (#57) Add worktree bootstrap for local configuration (#56) Make thread opens part of navigation history (#55) Remove the decoded-avatar hot set; respect Save-Data (#60) Add CODEOWNERS reviewers (#61) Add guarded Workflows page and session capability (#48) Signed-off-by: Clay Delk <clay.delk@gmail.com> # Conflicts: # dev/relay-broker.mjs # src/bundled/channels/ChannelsPage.tsx # src/features/messages/ChannelTimeline.tsx # src/features/messages/MessageComposer.tsx # src/features/messages/MessageRow.tsx # src/features/messages/ThreadPanel.test.tsx # src/features/messages/ThreadPanel.tsx
Adds the bundled Links plugin with service icons, consistent link and mention styling, wrapped Markdown links, and clickable Buzz previews with author, avatar, timestamp, channel, and a four-line excerpt.
Links, mentions, channels, and emoji now render directly inside the composer. Copying and sending preserve the authored text and selected recipients. Pasted links add a space; mouse and keyboard selection include links; partial deletion returns a link to plain text.
Validation: code and design-system checks, 1,354 unit tests, 38 Node integration tests, plugin-manager tests, and the latest Chromium/WebKit composer journeys passed. Pre-push passed 331 related tests. Independent review found a paste/caret edge case, now fixed and covered in both engines. The broader local scan and fresh CI are still running; prior navigation-focus failures and requested reviews remain separate integration gates.