fix: route channel references across clients

[loganj]

🤖

Summary

Channel references and Buzz deep links now open their destination inside Buzz across mobile and desktop. The table below makes the platform contract explicit.

Platform behavior

Behavior	Platform	Result
#channel references	Mobile + Desktop	Opens the named channel when it resolves. Unknown names remain inert.
buzz://channel/<channel-uuid> rendered as a bare URL, <autolink>, or [labeled link](...)	Mobile + Desktop	Opens the channel in-app. Channel UUIDs are validated and normalized to lowercase.
buzz://message?channel=<uuid>&id=<event-id> rendered in those same three forms	Mobile + Desktop	Opens the channel at the referenced message.
Message links with &thread=<root-id>	Mobile + Desktop	Opens the referenced reply in its thread.
OS-launched channel, message, and thread links	Mobile + Desktop	Opens the same in-app destination. Desktop queues links received before the authenticated router mounts, so cold starts do not lose navigation.
Sentence punctuation after a bare Buzz URL	Mobile + Desktop	Remains prose rather than becoming part of the message ID, thread ID, or invite code. This PR adds mobile handling and preserves the existing desktop message-link behavior; desktop channel-link handling is added in parallel.
Rendered buzz://join?... links	Mobile only	Continues through the in-app join flow. Desktop rendered-message join links are unchanged.
OS-launched buzz://connect, buzz://join, and buzz://add-community links	Desktop only	Existing native community onboarding behavior is unchanged.

Malformed or unsupported buzz:// forms remain inert. Existing HTTP(S), media, external-link, and specialized navigation behavior is unchanged.

Related issue

None found.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dc16416431

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 685344ec0b

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f2ac29a872

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 49e91eed43

ℹ️ 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]

Requesting changes on Wes’s behalf.

[P2] Route rendered mobile channel URLs through the supplied channel callback.

MessageContent already computes resolvedChannelTap, and channel/thread pages supply onChannelTap through openChannelLink(... currentChannelId: ...) so links to the channel already being viewed do not push a duplicate detail page. The generic Buzz anchor handler instead sends every parsed buzz://channel/... through pendingDeepLinkProvider, bypassing that guard. Thus a pasted channel URL pushes a duplicate page while the equivalent #channel reference correctly no-ops.

Please dispatch ChannelDeepLink.channelId through resolvedChannelTap, retain the provider path for message/join links, and add regression coverage with an injected onChannelTap callback.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31ca79f5fe

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2c207eef2

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 363bc80f94

ℹ️ 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".

[brow]

🤖 Review of this PR at head 97dfaace980096a99cdc69d72fc277bbc513fd61. Comment only: no approve and no request-changes from us. Two independent agent reviews, one desktop and one mobile, plus verification of my own.

How to read the pins. This branch moved seven times during the review, so every claim below names the head it was measured at. Items marked construct-level were executed at an earlier head and re-checked at 97dfaace by confirming the named construct and its file blob are unchanged, rather than by re-running the suite. Concretely, and verified by blob hash at all three of c9065352, 363bc80f and 97dfaace: markdown/utils.ts, markdown/nodeCache.ts, markdown.tsx, markdown.test.mjs, channelLink.ts, remarkChannelDeepLinks.ts, ChannelDeepLink.tsx, useMessageDeepLinks.ts and deep_link.dart are all identical, and message_content.dart is identical from d2c207ee onward. So every finding below except the two Rust/drain notes sits on a blob that has not moved since it was measured. The last two pushes touched only the navigation-drain reset path.

Validation was pnpm install / pnpm test / pnpm typecheck for desktop and flutter analyze plus the FULL host-VM flutter test for mobile. No simulator or device run, so anything that needs one is called out as unverified at the end.

No blockers. The earlier duplicate-navigation blocker is fixed and the fix is load-bearing: rendered ChannelDeepLink targets go through resolvedChannelTap, and replacing that with the old pending-provider route turns the suite red at the existing channel-callback test (1178 passed / 1 failed), so the test is real coverage rather than decoration. That routing is confirmed present at 97dfaace by blob identity. Suite counts: mobile 1179 passed / 0 failed with flutter analyze clean, and desktop 4202 passed / 0 failed across 63 suites with tsc --noEmit clean, both executed at c9065352 and construct-level at the head per the blob table above.

One thing we would fix before merge, in the description rather than the code

The description says sentence punctuation after a bare Buzz URL "adds the missing mobile guard; desktop already handled it." That is accurate for message links and misleading for channel links. Desktop's message-link peel does predate this PR (trimMessageLinkMatch and its remark plugin are blob-identical at the merge base, and a 29-probe executed sweep over . , ! ? : ; ) ], space, newline, .., .), ). and (see <url>) passes at the base). But desktop's channel recognizer does not exist at the merge base at all: channelLink.ts, remarkChannelDeepLinks.ts and ChannelDeepLink.tsx are absent there, and the only buzz://channel strings in desktop/ at the base route to the external opener. So desktop's channel-link peel is added by this PR, in parallel with mobile's, not inherited. Since the mobile change is justified by parity with desktop, that one sentence is worth correcting.

A dead tap that is new in this PR: bare Buzz URLs inside non-adjacent emphasis

Measured in real Dart, by widget probe through MessageContent, at two successive heads (74fe972f and d2c207ee). message_content.dart is the same blob at 97dfaace, so this is current.

The delimiter peel only fires when the matching opening delimiter immediately precedes the URL. So the adjacent forms work correctly: **<url>**., ***<url>***!, ~~<url>~~, and *<url>* all route to the channel. But when any text sits between the opener and the URL, or the emphasis is nested, the closing delimiter is swallowed into the URL: *join <url>*, **join <url>**, _join <url>_, ~~join <url>~~, **_<url>_**.. The channel-id parse then rejects the corrupted target, and the tap does nothing at all: no navigation and nothing queued for the dispatcher. A user sees underlined link text that is silently inert.

This is new in this PR, not pre-existing. At the merge base both mobile autolink steps matched https?:// only, so a bare buzz:// URL was never turned into a link and there was no tap to lose. Linkifying the custom scheme is what created the case. The fix is presumably the same shape the existing code already uses, applied to the closing delimiter rather than only to the immediately-preceding opener. A related bound worth knowing: the bare-URL pattern's negative lookbehind excludes (, so a bare Buzz URL right after an opening parenthesis is still not linkified.

Findings we chased and are NOT charging to this PR

Each of these was measured at the head and then controlled against the merge base, because pre-existing and introduced-here are different verdicts.

• Mutex expect() on the navigation queue: not reproduced as filed. The shipped queue's private lock() recovers from poisoning via into_inner, all three call sites use it, and there is a regression test that genuinely poisons the lock from a panicking thread. The residue is pre-existing: the older community queue still has production expect() calls, and the expect( count in deep_link.rs is 9 at the merge base, 9 at 363bc80f and 9 at 97dfaace. If one ever fired, the consequence is a rejected IPC promise that strands a queued connect/join until relaunch, not a crash. Worth a follow-up, not a change here.
• Channel links arriving before the router listener mounts: this PR is the fix, not the cause. At the merge base there is no navigation queue at all, so early links were simply dropped. At the head both arms queue before emitting, the frontend drains, and an item is only dropped after an explicit accept plus acknowledge.
• Acknowledging before navigation resolves: addressed, and pinned. Both callbacks now await the navigation and return true only after it resolves, and the drain gates on acceptance before acknowledging. Removing that gate turns the suite red at the FIFO-teardown test, so the fix has real coverage.
• Cross-community deep links. A link to a channel outside the active community navigates and then shows an empty pane on desktop (no error, no offer to switch community); mobile surfaces its existing not-found message and also does not switch. This is the same behavior as the merge base for message links, so it is not a regression, and we understand community switching to be deliberately out of scope here. Recording it as a known gap.

Coverage gaps: three edits that no test notices

Mutants run against the full desktop suite with tsc --noEmit also clean, so these are behavioral holes rather than compile-checked ones, and each was confirmed observable against a pristine control through the production export. All three files are blob-identical at 97dfaace.

• Removing the channel-link case from the shared markdown urlTransform leaves the suite fully green. The cause is duplication: markdown.test.mjs re-implements its own copy of that transform instead of importing the production one, so no test in src/ exercises the real symbol. Against the production export, the pristine version returns the channel href unchanged and the mutant returns an empty string, i.e. a dead link.
• Removing remarkChannelDeepLinks from the plugin list leaves the suite fully green (type-preserving mutant, so this is not just an unused-import error). Against the production render path, pristine emits the channel node and the mutant renders the raw URL as plain text, so bare channel URLs would silently stop being clickable with zero test failure.
• useMessageDeepLinks has no unit test. Reverting its await to fire-and-forget survives, and even making the hook do nothing at all leaves the suite green. The await/acknowledge decision is tested in the shared module; its use in the hook is not.

Two small notes on the newest two commits

These landed while the review was being written, so they are read-level observations at 97dfaace rather than mutation-tested claims, and neither is a blocker.

• The generation guard plus the awaited clear_pending_navigation_deep_links addresses the acknowledge half and the native-head half of the reported reset hazard. navigationDrainTail is still module-level and is not reset, so an invalidated drain still occupies the serialization tail until it returns. Since the guard makes it return at the next checkpoint rather than continue, we did not find a lasting head-of-line stall, but we did not prove its absence either: the new test hand-calls the reset and asserts non-acknowledgement plus clearCount === 1, which does not exercise ordering against a concurrent new-community drain.
• The Rust queue test previously asserted assert!(queue.acknowledge("second")) and now calls queue.clear() in that spot. clear() is worth covering, but the replaced line was the assertion that a second acknowledge actually succeeds. Consider keeping both rather than substituting one for the other.

Not verified

No simulator, device, or browser run: this review deliberately ran without iOS tooling, and we did not run the Playwright suite locally because its fixed dev-server port can serve another checkout's build and produce a false green. So the two new cold-start end-to-end tests were reviewed by reading only, real OS-level app-link delivery is unverified on both platforms, and the Rust queue tests were left to CI rather than run locally.

CI status: at the previous head 363bc80f CI was terminal green, read at job level rather than off the summary (run completed/success, 22 jobs with 16 success, 6 skipped, 0 failed, 0 running, Desktop Release Candidate success, and all 26 check-runs success or skipped). At 97dfaace the CI run was still in_progress when this review was written (Desktop Release Candidate already success), so that is a statement of timing and not a claim either way about the newest commit.

[brow]

🤖 Addendum to our review above, same head 97dfaace980096a99cdc69d72fc277bbc513fd61. One of our seats finished a mutation sweep on the two newest commits after we posted, and it produced one item we would want a maintainer to look at plus two corrections to what we said. Still comment only.

Correction first, because it cuts against us. Our review said we "found no lasting head-of-line stall but did not prove its absence" for the unreset navigationDrainTail. That was too soft in one direction and too vague in the other. Measured now with one variable changed and identical probe text against both blobs: head-of-line blocking is real and unchanged by this PR. A drain from the previous community still occupies the shared tail (blockedWhileAInFlight true at both d2c207ee and 363bc80f), and a route whose promise never settles strands the next community's item permanently at both revisions. Because the behavior is byte-for-byte the same before and after, this is pre-existing and not yours to fix here. It needs a permanently-hanging navigation to bite, since the consumer awaits the route. Recording it as a follow-up, not a change request.

The one item we would put in front of you before merge. resetNavigationDeepLinkDrain is now async and awaits invoke("clear_pending_navigation_deep_links"). That made resetCommunityState async, and its call site is an await that sits outside any try/catch, inside an init() that is launched as void init(). The applyCommunity try/catch begins after it, so it does not cover this. If that IPC call ever rejects, the switch aborts after relayClient.disconnect() has already run: applyCommunity is never called and the ready state is never set, so the app parks on the switch gate with the relay torn down, and the rejection surfaces nowhere. Measured in the faithful caller shape (rejectionPropagates true, appliedCommunity false, becameReady false), with a resolving control that throws nothing. Note also that this invoke has no isTauri() guard, unlike the guarded call a couple of lines below it in the same file.

We did not establish that an IPC rejection is reachable in production, which is a Tauri-runtime question we had no budget to answer here, so we are calling this needs-attention rather than a proven defect. It is cheap to make unconditional: one catch around the invoke, or making it fire-and-forget, removes the class regardless of reachability. We flag it because the current shape fails silently rather than loudly.

Two coverage notes on the new test, which is otherwise good. Type-preserving single-clause mutants, each applied alone, full package suite every time (pristine control 4203 passed / 0 failed, tsc --noEmit rc=0 at this head; two of us reproduced the control independently in separate checkouts).

• The new test pins the post-accept generation re-check (dropping it goes red, 4202/1) and the native clear (dropping it goes red on clearCount). It is blind to the other two guard clauses: dropping the post-take re-check and replacing the loop condition with while (true) both leave 4203 passed / 0 failed. Those are genuine holes rather than equivalent mutants, shown by landing a reset inside each specific await window: pristine routes nothing, the post-take mutant routes the stale item, and the loop mutant takes a second item it should not.
• More useful: the production wiring itself is untested. Replacing the await resetNavigationDeepLinkDrain() call inside resetCommunityState with a no-op reference leaves the suite at 4203 passed / 0 failed with tsc rc=0, because the test calls the helper by hand. So the test pins the helper's behavior, not that anything ever invokes it on a community switch. Same shape as the useMessageDeepLinks gap in our review: the decision is covered, its use is not.

One path where the guard never bumps. The only production call site is resetCommunityState, which is gated on having already initialized. The onboarding-cancel path that clears communities drives the hook into its no-active-community branch and returns before the reset, so no bump happens there. A generation guard whose bump is missing on one path is exactly what a test with a hand-called reset cannot catch.

Our disposition does not change. Everything above is either pre-existing, coverage, or an unproven-reachability hazard with a one-line mitigation, so the recommendation stays as it was: the deep-link behavior itself is sound and test-pinned at this head.

[brow]

🤖 Correction to our own earlier comment. Measured at 97dfaace980096a99cdc69d72fc277bbc513fd61.

We withdraw one sentence from our addendum. It ended with: "Note also that this invoke has no isTauri() guard, unlike the guarded call a couple of lines below it in the same file." That comparison is wrong, and it was unfair to this change.

isTauri does not appear in desktop/src/shared/deep-link.ts at all. The guard we were pointing at is in a different file: it is the isTauri() && isMacPlatform() check around clearTrayAgentActivity() in useCommunityInit.ts, a few lines below the resetNavigationDeepLinkDrain() call there. We had both files open at the same construct and conflated them. Flagging the file explicitly rather than just deleting the sentence, so that anyone who greps deep-link.ts for isTauri and finds nothing knows the error was ours.

The comparison also cut the wrong way. Every invoke call site in deep-link.ts is unguarded, including the two that predate this PR, which are unguarded at the merge base as well. So an unguarded invoke in that file is the existing convention there, and the new call is consistent with its neighbours rather than a deviation. Our sentence implied the opposite.

The needs-attention item itself is unchanged, because it never rested on a platform guard. What distinguishes the new call is the caller, not the guard: the pre-existing invokes are awaited inside the listener and drain paths, while this one is awaited from resetCommunityState, whose caller in useCommunityInit.ts awaits it outside any try/catch, inside an init() launched as void init(), with the applyCommunity try/catch not opening until after it. That is what makes a rejection able to abort the community switch after relayClient.disconnect() has already run, with nothing surfacing. Substitute that contrast for the withdrawn sentence.

Everything else in that comment stands as measured, and its bound is unchanged: we did not establish that an IPC rejection is reachable here, so this remains a needs-attention item and not a defect, with the same one-line mitigation (a catch around the invoke, or not awaiting it). Our recommendation is still approve with nits.

[loganj]

Addressed the latest maintainer review at b68ad08dc1ea6b46551ae02522432ef07d90b5b6 (feedback fix e51441a, then clean merge of current origin/main required by the branch-skew gate).

• Bare mobile buzz:// links now keep non-adjacent/nested Markdown closing delimiters outside the URL and remain tappable.
• Native queue clearing is best-effort, so IPC rejection cannot abort community switching; generation invalidation still prevents stale acknowledgement.
• Regressions cover post-take and loop generation checkpoints, no-active-community reset wiring, production switch IPC invocation, and both FIFO acknowledgement and clear behavior.
• Independent exact-head review approved both the fix and merged head with no blockers.
• Push hooks passed all desktop/mobile/Rust gates. Local, remote, and PR heads match.

@wesbillman re-review requested when convenient.

Addressed: rendered ChannelDeepLink targets now dispatch through resolvedChannelTap, message/join targets retain the pending-provider path, and regression coverage verifies the injected callback. Exact head 7c4ad10 is independently approved and CI is green.