diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md new file mode 100644 index 00000000000..5962920e0f3 --- /dev/null +++ b/PR_DESCRIPTION.md @@ -0,0 +1,197 @@ +# desktop: channel-feature registry (settings sections + channel classification) + +Implements #3280. + +## Problem + +Adding a channel-scoped feature to the desktop client means editing several +centrally-owned files at once. Two concrete cases of this exist in current +upstream `main` today: + +1. **Settings-section wiring** (`SettingsPanels.tsx` / `SettingsView.tsx`): + a `SettingsSection` union type, a `SETTINGS_SECTION_VALUES` array, + `isSettingsSection`, a `settingsSections` descriptor array, a + `renderSettingsSection` switch with a `never` exhaustiveness gate, and + `SettingsView`'s separate `settingsNavGroups` map. Five parallel + structures a new settings section has to touch. +2. **Channel classification is scattered.** `ChatHeader`'s `ChannelIcon` + re-derives "what kind of channel is this" (dm, private, forum, hash) as + an inline if-chain, and `ChannelScreen` separately re-checks + `activeChannel.channelType === "forum"` in four different places to + decide what to render. + +This PR is the client-side companion the RFC describes, ported fresh onto +current upstream. Our original implementation lived in a fork whose git +history isn't connected to this repo (a content-snapshot import, not a real +fork), so it couldn't be cherry-picked. See "Provenance" below. + +## How + +**Commit 1, settings-section registry.** `settingsSections` is now the +single source of truth: each descriptor carries `value`/`label`/`icon`/ +`featureGate` as before, plus `group`, `order`, and a `render(props)` +closure lifted from the old switch case. `SettingsView` derives nav +grouping and panel rendering directly from the registry. Behavior, +`data-testid`s, and section order/grouping are unchanged, including the +pre-existing `"moderation"` section, which isn't wired into any nav group +either before or after this change. + +**Commit 2, `shared/channel-features` registry.** A `ChannelFeaturePlugin` +registry modeled on the existing `shared/features/` flag manifest (typed +definition list, resolver hook, gate): + +```ts +interface ChannelFeaturePlugin { + id: string; + parseBinding: (channel: ChannelClassifyInput) => T | null; + glyph?: LucideIcon; + priority?: number; // lower runs first; ties keep registration order +} +``` + +Four built-in plugins (`dm`, `private-channel`, `forum`, `stream`, +registered in priority order in `builtins.ts`) reproduce `ChatHeader`'s +existing dm, private, forum, hash cascade exactly, and now back two call +sites that used to derive it independently: + +- `ChatHeader`'s `ChannelIcon` calls `channelGlyph({channelType, visibility})` + instead of the inline if-chain. +- `ChannelScreen` computes `isActiveChannelForum` once, via + `classifyChannel(activeChannel)?.pluginId === "forum"`, and reuses it for + the forum/chat content dispatch, the single-panel-view check, the + transparent-chrome check, the timeline-loading gate, and the "manage + channel" action's forum branch. Five sites that used to check + `channelType === "forum"` on their own. + +Registration happens at module scope in `shared/channel-features/index.ts`, +the same way `shared/features/manifest` loads at import time, so any call +site that imports from the barrel gets the built-ins for free. + +## Not in this PR + +The RFC's fuller proposed surface, `tabs`, `settingsPanel`, `sidebar` +group/create-actions, `headerAction`, is not in this PR. Our original +implementation had those because our fork added new channel types +(product/repo/board-hierarchy) that needed their own tab bars, sidebar +groups, and settings panels. Current upstream `main` has no second +consumer yet: the only two dispatch points that exist today (`ChatHeader`'s +glyph, `ChannelScreen`'s forum/chat split) are a binary classification, not +a multi-tab surface. Adding the tab/sidebar-group machinery now would be +speculative for zero real callers. See Follow-ups below for what would +justify porting the rest. + +## Relationship to #3275 + +#3275 ("host MCP Apps as channel tabs") is the motivating case for the RFC: +it extends `ChannelScreen`'s shared shells directly to add a new tab type. +This PR doesn't touch #3275's code, and its content dispatch (forum vs. +chat) is orthogonal to MCP-App tabs, which install within a channel that's +already classified as a normal chat channel. There's no merge conflict or +ordering dependency between the two. + +The natural follow-up once both land: an MCP-App-tabs plugin would extend +`ChannelFeaturePlugin` with a `tabs` field, as the RFC describes, and +register its tab bar for channels with an installed app. This PR doesn't +ship that field pre-emptively, since there's no real second tab-contributing +plugin in this repo yet to shape it against. It answers the same question +by proving the `parseBinding`/`glyph`/priority-cascade mechanics on two real +call sites, so that field has something real to build on. + +## Review focus + +1. Is the reduced scope (settings-section registry plus a two-plugin + channel classifier, no `tabs`/`sidebar`/`headerAction` surface) the + right size for a first PR, or should more of the RFC's surface be + included speculatively even without a second consumer? +2. Does this typed classification seam compose cleanly with #3275's own + review question about the same relationship ("does the current typed + channel-surface seam compose cleanly with the behavior-preserving + registry proposed in #3280")? This PR doesn't answer that by adding a + `tabs` field, it answers it by proving the underlying mechanics: does + that stand on its own, or do you want the `tabs` field sketched here + too even without a plugin to use it yet? +3. `channelGlyph`'s stream-vs-Hash styling special case in `ChatHeader.tsx` + is the one place the port kept a pre-existing wart rather than + generalizing it. Worth fixing here, or fine as a named follow-up? + +## Testing + +At commit `31861518d` (the last code commit on this branch; later commits +only touch this description): + +- `pnpm typecheck`: clean. +- `pnpm lint` (`biome check`): clean on all touched/added files (two + pre-existing warnings and two pre-existing infos elsewhere in the repo, + unrelated and unchanged). +- `pnpm test` (unit, `node --test`): 5463 passed, 0 failed, including the + new `desktop/src/shared/channel-features/registry.test.mjs` (11 tests: + built-in plugin registration order, idempotent re-registration, the + dm/private/forum/stream classification cascade and its precedence, and + `registerChannelFeature`'s sort/dedup behavior). +- `pnpm build:e2e`: clean build. +- `pnpm exec playwright test --project=smoke --grep "forum|Forum|settings|Settings|sidebar|Sidebar"`: + 112 passed, 1 skipped, 0 failed (covers `settings-section-layout.spec.ts`, + `sidebar.spec.ts`, `sidebar-offcanvas-rail.spec.ts`, + `sidebar-relay-card.spec.ts`, `sidebar-snapshot.spec.ts`, + `sidebar-more-unread-overlap.spec.ts`, + `hosted-communities-settings-screenshots.spec.ts`, + `invites-settings-screenshots.spec.ts`, `profile-backup-settings.spec.ts`, + `voice-settings.spec.ts`, and the forum-touching cases in the broader + channel specs). +- Did not run the full Playwright suite or the Rust side (`just ci`). This + PR has no Rust changes and no diff outside `desktop/src`, and the full + JS/TS suite plus the targeted grep above already cover every spec that + touches channel tabs, sidebar hierarchy, and settings. A reviewer with + more time may want the full suite for extra confidence. + +### Screenshots + +Captured locally against a dev build and the mock bridge: + +- Settings, Appearance panel, showing the registry-rendered nav groups + (Personal/Communities/App) and unchanged panel content. +- The `#general` channel, showing the `stream` plugin's Hash glyph in the + channel header, the same icon the old inline cascade produced for a + plain open stream channel. + +These weren't posted through `scripts/post-screenshots.sh`, since that +script needs an open PR and none was opened for this branch. Whoever opens +the PR from `upstream-pr/channel-feature-registry-seam-b` should recapture +and post screenshots through that script at PR-creation time. + +## Follow-ups + +- Port the `tabs`/`ChannelFeatureShell` surface once a second real + tab-contributing plugin exists (an MCP-App-tabs plugin building on + #3275, or a future Sequence/board/docs-style channel type). A second + concrete caller is the right trigger, shaping it against one hypothetical + consumer risks guessing wrong. +- Sidebar group/create-action surface (`ChannelFeatureSidebar`), same + reasoning. Upstream has no repo/product-style sidebar hierarchy today to + drive the design. +- `headerAction`, same reasoning. No plugin needs to contribute a header + action yet (our fork needed this for a "New idea" dialog action that + doesn't exist upstream). +- Consider whether `channelGlyph`'s stream-vs-Hash styling special case in + `ChatHeader.tsx` is worth generalizing into a per-plugin style hook, or + left as the one acknowledged wart of an otherwise uniform glyph lookup. + +## Duplicate check + +`gh api search/issues -f q='repo:block/buzz channel feature registry ChannelFeaturePlugin'` +returns exactly two results: issue #3280 (this RFC) and PR #3275 (the MCP +Apps host, discussed above). No other open or closed issue or PR implements +a channel-feature or settings-section registry for the desktop client. + +## Provenance + +The design was originally implemented and reviewed in a fork whose git +history isn't connected to this upstream repo (a content-snapshot import, +not a real fork), so it couldn't be cherry-picked or rebased. This PR is a +fresh port of the mechanism onto current `upstream/main`, adapted to what +upstream's `ChannelScreen`/`ChatHeader`/settings code actually look like +today, which has drifted from our fork's snapshot: the settings-section +descriptor shape, the channel dispatch's actual branch points, and the +absence of any custom channel-type/tab surface all differ from what the +original diff assumed. See "Not in this PR" above for the scope that drift +and the behavior-preserving constraint together produced. diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 240a9ad70c1..c7ebfed5a08 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -77,6 +77,7 @@ import { useElementWidth } from "@/shared/hooks/use-mobile"; import { useThreadPanelWidth } from "@/shared/hooks/useThreadPanelWidth"; import { AUXILIARY_PANEL_SINGLE_COLUMN_BREAKPOINT_PX } from "@/shared/layout/AuxiliaryPanel"; import { normalizePubkey } from "@/shared/lib/pubkey"; +import { classifyChannel } from "@/shared/channel-features"; import { useChannelActivityTyping } from "./useChannelActivityTyping"; import { useChannelAgentSessions } from "./useChannelAgentSessions"; import { useMessageProfiles } from "./useMessageProfiles"; @@ -171,6 +172,12 @@ export function ChannelScreen({ const mainInsetRef = useMainInsetRef(); const currentPubkey = currentIdentity?.pubkey; const activeChannelId = activeChannel?.id ?? null; + // Classifies through the same channel-feature registry ChatHeader's glyph + // resolution uses (see `shared/channel-features`), rather than each call + // site below independently re-checking `channelType === "forum"`. + const isActiveChannelForum = + activeChannel !== null && + classifyChannel(activeChannel)?.pluginId === "forum"; const isHuddleTranscript = useIsHuddleTranscript(activeChannelId); const relaySelfPubkey = useRelaySelfQuery(activeChannel !== null).data; const requireThreadEditResolutionRef = React.useRef<() => boolean>( @@ -624,7 +631,7 @@ export function ChannelScreen({ activeChannelId !== null && settledChannelIdRef.current === activeChannelId; const timelineLoadingNow = activeChannel !== null && - activeChannel.channelType !== "forum" && + !isActiveChannelForum && selectTimelineLoadingState( { isPending: messagesQuery.isPending, @@ -709,9 +716,7 @@ export function ChannelScreen({ channelContentWidthPx > 0 && channelContentWidthPx < AUXILIARY_PANEL_SINGLE_COLUMN_BREAKPOINT_PX; const isSinglePanelView = - isNarrowPanelViewport && - activeChannel?.channelType !== "forum" && - hasAuxiliaryPanel; + isNarrowPanelViewport && !isActiveChannelForum && hasAuxiliaryPanel; const shouldCompactHeaderActions = hasAuxiliaryPanel && channelContentWidthPx > 0 && @@ -724,7 +729,7 @@ export function ChannelScreen({ }); const handleManageChannel = React.useCallback(() => { if (!requireThreadEditResolution()) return; - if (activeChannel?.channelType === "forum") { + if (isActiveChannelForum) { openGlobalChannelManagement(); return; } @@ -740,7 +745,7 @@ export function ChannelScreen({ setProfilePanelPubkey(null); setChannelManagementOpen(true); }, [ - activeChannel?.channelType, + isActiveChannelForum, channelManagementOpen, openGlobalChannelManagement, requireThreadEditResolution, @@ -772,11 +777,12 @@ export function ChannelScreen({ onManageChannel={handleManageChannel} onToggleMembers={handleToggleMembers} showHeaderContent={!isSinglePanelView && !isHuddleTranscript} - transparentChrome={activeChannel?.channelType !== "forum"} + transparentChrome={!isActiveChannelForum} /> ), [ activeChannel, + isActiveChannelForum, activeChannelEphemeralDisplay, activeChannelTitle, shouldCompactHeaderActions, @@ -824,7 +830,7 @@ export function ChannelScreen({ ref={channelContentRef} > {activeChannel ? ( - activeChannel.channelType === "forum" ? ( + isActiveChannelForum ? ( ; } - if (channelType === "dm") { - return ; + // dm → private → forum → hash, in that order, is now a priority-ordered + // channel-feature plugin cascade (see `shared/channel-features/builtins.ts`) + // rather than an inline if-chain — `ChannelScreen`'s forum/chat content + // dispatch classifies through the same plugins, so there's one place that + // knows "what kind of channel is this" instead of two independently + // re-deriving it. + if (!channelType) { + return ; } - - if (visibility === "private") { - return ; - } - - if (channelType === "forum") { - return ; - } - - return ; + const Glyph = + channelGlyph({ channelType, visibility: visibility ?? "open" }) ?? Hash; + return Glyph === Hash ? ( + + ) : ( + + ); } export function ChatHeader({ diff --git a/desktop/src/features/settings/ui/SettingsPanels.tsx b/desktop/src/features/settings/ui/SettingsPanels.tsx index 6b00fc8f74c..089045031d5 100644 --- a/desktop/src/features/settings/ui/SettingsPanels.tsx +++ b/desktop/src/features/settings/ui/SettingsPanels.tsx @@ -81,58 +81,69 @@ import { UpdateChecker } from "../UpdateChecker"; import { SettingsSectionHeader } from "./SettingsSectionHeader"; import { VoiceSettingsCard } from "./VoiceSettingsCard"; -export type SettingsSection = - | "profile" - | "notifications" - | "voice" - | "experimental" - | "agents" - | "channel-templates" - | "compute" - | "appearance" - | "shortcuts" - | "hosted-communities" - | "community-members" - | "moderation" - | "custom-emoji" - | "local-archive" - | "mobile" - | "updates"; +/** + * A settings section identifier. `settingsSections` below is now the single + * source of truth for which section values exist, what each one renders, and + * where it sits in navigation — there is no separate union type to keep in + * sync. This alias stays a plain `string` purely so existing callers can + * still annotate a variable's intent; validity is checked at runtime via + * `isSettingsSection`, which consults the registry directly. + * + * Previously, adding a section meant touching a `SettingsSection` union, a + * `SETTINGS_SECTION_VALUES` array, a `settingsSections` descriptor, a + * `renderSettingsSection` switch case, and a `settingsNavGroups` entry (5 + * parallel structures). It's now a single `settingsSections` entry. + * + * The `never` exhaustiveness check a switch statement used to provide is + * replaced by the registry being the *only* place a section is defined: + * there is nothing left to fall out of sync with. + */ +export type SettingsSection = string; export const DEFAULT_SETTINGS_SECTION: SettingsSection = "profile"; -const SETTINGS_SECTION_VALUES: readonly SettingsSection[] = [ - "profile", - "notifications", - "voice", - "experimental", - "agents", - "channel-templates", - "compute", - "appearance", - "shortcuts", - "hosted-communities", - "community-members", - "moderation", - "custom-emoji", - "local-archive", - "mobile", - "updates", -]; - export function isSettingsSection(value: unknown): value is SettingsSection { return ( - typeof value === "string" && - (SETTINGS_SECTION_VALUES as readonly string[]).includes(value) + typeof value === "string" && settingsSections.some((s) => s.value === value) ); } +/** Nav-group identifier a settings section is grouped under. See {@link SETTINGS_NAV_GROUPS}. */ +export type SettingsNavGroupId = "personal" | "communities" | "app"; + +/** + * Static nav-group order + labels for the settings sidebar. Section → group + * membership lives on each `SettingsSectionDescriptor.group`; this just + * controls the order the group headers themselves appear in and what + * they're labeled. + */ +export const SETTINGS_NAV_GROUPS: ReadonlyArray<{ + id: SettingsNavGroupId; + label: string; +}> = [ + { id: "personal", label: "Personal" }, + { id: "communities", label: "Communities" }, + { id: "app", label: "App" }, +]; + export type SettingsSectionDescriptor = { value: SettingsSection; label: string; icon: LucideIcon; + /** + * Nav group this section is grouped under. Omitted for a section that + * should stay reachable by value but not appear in the sidebar — e.g. + * "moderation" below, which predates this registry and was never wired + * into a nav group; that (likely unintentional) behavior is preserved + * as-is here rather than fixed as a drive-by in this refactor. + */ + group?: SettingsNavGroupId; + /** Explicit ordering within the group; lower renders first. Defaults to 0. */ + order?: number; /** If set, this section is only visible when the feature is enabled */ featureGate?: string; + /** Renders this section's panel body. */ + render: (props: SettingsPanelProps) => React.ReactNode; }; export type SettingsPanelProps = { @@ -151,88 +162,162 @@ export type SettingsPanelProps = { }; export const settingsSections: SettingsSectionDescriptor[] = [ - { - value: "appearance", - label: "Appearance", - icon: MonitorCog, - }, + // --- Personal --- { value: "profile", label: "Profile", icon: UserRound, + group: "personal", + order: 0, + render: (props) => ( + + ), + }, + { + value: "appearance", + label: "Appearance", + icon: MonitorCog, + group: "personal", + order: 1, + render: () => , }, { value: "notifications", label: "Notifications", icon: BellRing, + group: "personal", + order: 2, + render: (props) => ( + + ), }, { value: "voice", label: "Voice", icon: Volume2, + group: "personal", + order: 3, + render: () => , }, { - value: "experimental", - label: "Experiments", - icon: FlaskConical, + value: "shortcuts", + label: "Shortcuts", + icon: Keyboard, + group: "personal", + order: 4, + render: () => , }, { - value: "agents", - label: "Agents", - icon: Bot, - featureGate: "managed-agents", + value: "custom-emoji", + label: "Custom emoji", + icon: Smile, + group: "personal", + order: 5, + featureGate: "custom-emoji", + render: () => , + }, + { + value: "local-archive", + label: "Local archive", + icon: Archive, + group: "personal", + order: 6, + render: () => , }, { value: "channel-templates", label: "Channel templates", icon: LayoutTemplate, + group: "personal", + order: 7, featureGate: "channel-templates", + render: () => , }, - { - value: "compute", - label: "Compute", - icon: Cpu, - }, - { - value: "shortcuts", - label: "Shortcuts", - icon: Keyboard, - }, + // --- Communities --- { value: "hosted-communities", label: "Hosted communities", icon: MessagesSquare, + group: "communities", + order: 0, + render: () => , }, { value: "community-members", label: "Invites", icon: Ticket, + group: "communities", + order: 1, + render: (props) => ( + + ), }, + // --- Not wired into any nav group (see doc comment on `group` above) --- { value: "moderation", label: "Moderation", icon: ShieldAlert, + render: () => , }, + // --- App --- { - value: "custom-emoji", - label: "Custom emoji", - icon: Smile, - featureGate: "custom-emoji", + value: "agents", + label: "Agents", + icon: Bot, + group: "app", + order: 0, + featureGate: "managed-agents", + render: () => , }, { - value: "local-archive", - label: "Local archive", - icon: Archive, + value: "compute", + label: "Compute", + icon: Cpu, + group: "app", + order: 1, + render: () => , + }, + { + value: "experimental", + label: "Experiments", + icon: FlaskConical, + group: "app", + order: 2, + render: () => , }, { value: "mobile", label: "Mobile", icon: Smartphone, + group: "app", + order: 3, + render: (props) => ( + + ), }, { value: "updates", label: "Updates", icon: Download, + group: "app", + order: 4, + render: () => , }, ]; @@ -798,69 +883,3 @@ function ThemeSettingsCard() { ); } - -export function renderSettingsSection( - section: SettingsSection, - props: SettingsPanelProps, -): React.ReactNode { - switch (section) { - case "profile": - return ( - - ); - case "notifications": - return ( - - ); - case "voice": - return ; - case "experimental": - return ; - case "agents": - return ; - case "channel-templates": - return ; - case "compute": - return ; - case "appearance": - return ; - case "shortcuts": - return ; - case "hosted-communities": - return ; - case "community-members": - return ( - - ); - case "moderation": - return ; - case "custom-emoji": - return ; - case "local-archive": - return ; - case "mobile": - return ; - case "updates": - return ; - default: { - const exhaustiveCheck: never = section; - return exhaustiveCheck; - } - } -} diff --git a/desktop/src/features/settings/ui/SettingsView.tsx b/desktop/src/features/settings/ui/SettingsView.tsx index 2f9b2c36a1a..cad004c9785 100644 --- a/desktop/src/features/settings/ui/SettingsView.tsx +++ b/desktop/src/features/settings/ui/SettingsView.tsx @@ -30,7 +30,7 @@ import { } from "@/shared/ui/sidebar"; import { SidebarMenuLabel } from "@/shared/ui/sidebar-menu-label"; import { - renderSettingsSection, + SETTINGS_NAV_GROUPS, settingsSections, type SettingsPanelProps, type SettingsSection, @@ -48,33 +48,6 @@ type SettingsViewProps = SettingsPanelProps & { section: SettingsSection; }; -const settingsNavGroups: Array<{ - label: string; - sections: SettingsSection[]; -}> = [ - { - label: "Personal", - sections: [ - "profile", - "appearance", - "notifications", - "voice", - "shortcuts", - "custom-emoji", - "local-archive", - "channel-templates", - ], - }, - { - label: "Communities", - sections: ["hosted-communities", "community-members"], - }, - { - label: "App", - sections: ["agents", "compute", "experimental", "mobile", "updates"], - }, -]; - function SettingsSectionButton({ active, onSelect, @@ -186,23 +159,32 @@ export function SettingsView({ return () => window.removeEventListener("keydown", handleKeyDown); }, [onClose]); - const visibleSectionByValue = React.useMemo( - () => new Map(visibleSections.map((entry) => [entry.value, entry])), - [visibleSections], - ); - const visibleNavGroups = React.useMemo( - () => - settingsNavGroups - .map((group) => ({ - ...group, - sections: group.sections - .map((value) => visibleSectionByValue.get(value)) - .filter( - (entry): entry is SettingsSectionDescriptor => entry != null, - ), - })) - .filter((group) => group.sections.length > 0), - [visibleSectionByValue], + // Group visible sections by their `group` field, in `SETTINGS_NAV_GROUPS` + // order, sorted within each group by `order`. Sections with no `group` + // (e.g. "moderation") are omitted from the sidebar entirely, mirroring + // pre-registry behavior where they simply weren't listed in any nav group. + const visibleNavGroups = React.useMemo(() => { + const byGroup = new Map(); + for (const entry of visibleSections) { + if (!entry.group) continue; + const list = byGroup.get(entry.group); + if (list) { + list.push(entry); + } else { + byGroup.set(entry.group, [entry]); + } + } + return SETTINGS_NAV_GROUPS.map(({ id, label }) => ({ + label, + sections: (byGroup.get(id) ?? []) + .slice() + .sort((a, b) => (a.order ?? 0) - (b.order ?? 0)), + })).filter((group) => group.sections.length > 0); + }, [visibleSections]); + + const activeSection = React.useMemo( + () => visibleSections.find((entry) => entry.value === section), + [visibleSections, section], ); return ( @@ -342,7 +324,7 @@ export function SettingsView({ className="mx-auto flex min-h-full w-full max-w-4xl flex-col gap-4" data-testid={`settings-panel-${section}`} > - {renderSettingsSection(section, { + {activeSection?.render({ currentPubkey, fallbackDisplayName, isUpdatingDesktopNotifications, @@ -355,7 +337,7 @@ export function SettingsView({ onSetNotifyWhileViewing, onSetAllSlotAlertsEnabled, onSetSoundForSlot, - })} + }) ?? null} diff --git a/desktop/src/shared/channel-features/builtins.ts b/desktop/src/shared/channel-features/builtins.ts new file mode 100644 index 00000000000..26dbb945586 --- /dev/null +++ b/desktop/src/shared/channel-features/builtins.ts @@ -0,0 +1,42 @@ +import { CircleDot, FileText, Hash, Lock } from "lucide-react"; +import { registerChannelFeature } from "./registry"; + +/** + * The four channel-classification cases `ChatHeader`'s `ChannelIcon` used to + * check inline (dm → private → forum → hash), now expressed as + * priority-ordered channel-feature plugins. Registering them here — rather + * than inlining the checks at each call site — gives `ChatHeader` and + * `ChannelScreen` (which only needs the `forum` case, for its content + * dispatch) one shared source of truth for "what kind of channel is this", + * instead of two places independently re-deriving the same cascade. + * + * `stream` is an explicit catch-all (rather than leaving plain channels + * unclassified) so `classifyChannel` always returns a match once these are + * registered, matching `ChannelIcon`'s original `Hash`-by-default fallthrough. + */ +export function registerBuiltinChannelFeatures(): void { + registerChannelFeature({ + id: "dm", + priority: 0, + glyph: CircleDot, + parseBinding: (channel) => (channel.channelType === "dm" ? true : null), + }); + registerChannelFeature({ + id: "private-channel", + priority: 10, + glyph: Lock, + parseBinding: (channel) => (channel.visibility === "private" ? true : null), + }); + registerChannelFeature({ + id: "forum", + priority: 20, + glyph: FileText, + parseBinding: (channel) => (channel.channelType === "forum" ? true : null), + }); + registerChannelFeature({ + id: "stream", + priority: 30, + glyph: Hash, + parseBinding: () => true, + }); +} diff --git a/desktop/src/shared/channel-features/index.ts b/desktop/src/shared/channel-features/index.ts new file mode 100644 index 00000000000..efea53e72bc --- /dev/null +++ b/desktop/src/shared/channel-features/index.ts @@ -0,0 +1,21 @@ +import { registerBuiltinChannelFeatures } from "./builtins"; + +export { + channelGlyph, + classifyChannel, + classifyChannelWith, + getChannelPlugins, + registerChannelFeature, +} from "./registry"; +export type { + ChannelBinding, + ChannelClassifyInput, + ChannelFeaturePlugin, +} from "./types"; + +// Registering here (module scope, run once per module load thanks to ESM +// caching) means any call site that imports from this barrel gets the +// built-in dm/private-channel/forum/stream plugins for free, mirroring how +// `shared/features/manifest` loads its manifest at import time rather than +// requiring an explicit bootstrap call from `App.tsx`. +registerBuiltinChannelFeatures(); diff --git a/desktop/src/shared/channel-features/registry.test.mjs b/desktop/src/shared/channel-features/registry.test.mjs new file mode 100644 index 00000000000..12e7e6971f5 --- /dev/null +++ b/desktop/src/shared/channel-features/registry.test.mjs @@ -0,0 +1,115 @@ +import assert from "node:assert/strict"; +import { describe, it, beforeEach } from "node:test"; +import { CircleDot, FileText, Hash, Lock } from "lucide-react"; + +import { registerBuiltinChannelFeatures } from "./builtins.ts"; +import { + __resetChannelFeatureRegistryForTests, + channelGlyph, + classifyChannel, + getChannelPlugins, + registerChannelFeature, +} from "./registry.ts"; + +describe("built-in channel-feature plugins", () => { + beforeEach(() => { + __resetChannelFeatureRegistryForTests(); + registerBuiltinChannelFeatures(); + }); + + it("registers exactly the 4 built-in plugins, in priority order", () => { + const plugins = getChannelPlugins(); + assert.deepEqual( + plugins.map((p) => p.id), + ["dm", "private-channel", "forum", "stream"], + ); + }); + + it("re-registering is a no-op (idempotent)", () => { + registerBuiltinChannelFeatures(); + assert.equal(getChannelPlugins().length, 4); + }); + + const cases = [ + { + name: "a dm channel", + channel: { channelType: "dm", visibility: "open" }, + pluginId: "dm", + glyph: CircleDot, + }, + { + name: "a private stream channel", + channel: { channelType: "stream", visibility: "private" }, + pluginId: "private-channel", + glyph: Lock, + }, + { + name: "an open forum channel", + channel: { channelType: "forum", visibility: "open" }, + pluginId: "forum", + glyph: FileText, + }, + { + name: "a plain open stream channel", + channel: { channelType: "stream", visibility: "open" }, + pluginId: "stream", + glyph: Hash, + }, + ]; + + for (const { name, channel, pluginId, glyph } of cases) { + it(`classifies ${name} as "${pluginId}", matching ChatHeader's old ChannelIcon cascade`, () => { + const binding = classifyChannel(channel); + assert.ok(binding, "expected a non-null binding"); + assert.equal(binding.pluginId, pluginId); + assert.equal(channelGlyph(channel), glyph); + }); + } + + it("dm takes precedence over private (a dm can't be classified as private-channel)", () => { + const binding = classifyChannel({ + channelType: "dm", + visibility: "private", + }); + assert.equal(binding.pluginId, "dm"); + }); + + it("private takes precedence over forum", () => { + const binding = classifyChannel({ + channelType: "forum", + visibility: "private", + }); + assert.equal(binding.pluginId, "private-channel"); + }); +}); + +describe("registerChannelFeature", () => { + beforeEach(() => { + __resetChannelFeatureRegistryForTests(); + }); + + it("classifies against an empty registry as null", () => { + assert.equal( + classifyChannel({ channelType: "stream", visibility: "open" }), + null, + ); + }); + + it("sorts by priority, ties keeping registration order", () => { + registerChannelFeature({ id: "b", priority: 1, parseBinding: () => true }); + registerChannelFeature({ id: "a", priority: 0, parseBinding: () => true }); + registerChannelFeature({ id: "c", priority: 1, parseBinding: () => true }); + assert.deepEqual( + getChannelPlugins().map((p) => p.id), + ["a", "b", "c"], + ); + }); + + it("warns and ignores a duplicate id instead of throwing", () => { + registerChannelFeature({ id: "dup", parseBinding: () => true }); + assert.doesNotThrow(() => + registerChannelFeature({ id: "dup", parseBinding: () => null }), + ); + assert.equal(getChannelPlugins().length, 1); + }); +}); diff --git a/desktop/src/shared/channel-features/registry.ts b/desktop/src/shared/channel-features/registry.ts new file mode 100644 index 00000000000..c60cfb5d424 --- /dev/null +++ b/desktop/src/shared/channel-features/registry.ts @@ -0,0 +1,89 @@ +import type { + ChannelBinding, + ChannelClassifyInput, + ChannelFeaturePlugin, +} from "./types"; + +/** + * Module-level plugin list, kept sorted by `priority` (ties preserve + * registration order via `Array.prototype.sort`'s stability guarantee). + * + * Plugins are generic over their own binding type (`ChannelFeaturePlugin`), + * but the registry holds a heterogeneous mix, so entries are stored type-erased + * as `ChannelFeaturePlugin`. `registerChannelFeature` is the only + * place that performs the erasure (via `unknown` double-cast, never `any`) — + * every other module only ever sees the erased shape, which is exactly what + * `classifyChannel`'s `ChannelBinding` result needs. + */ +let plugins: ChannelFeaturePlugin[] = []; + +/** + * Register a channel-feature plugin. Duplicate `id`s are a no-op (warn + + * ignore) rather than a throw, so a stray double-registration (e.g. a hot + * reload or an accidental double barrel-import) can't crash app startup. + */ +export function registerChannelFeature( + plugin: ChannelFeaturePlugin, +): void { + if (plugins.some((existing) => existing.id === plugin.id)) { + console.warn( + `[channel-features] Duplicate channel feature id "${plugin.id}" — ignoring.`, + ); + return; + } + plugins = [ + ...plugins, + plugin as unknown as ChannelFeaturePlugin, + ].sort((a, b) => (a.priority ?? 0) - (b.priority ?? 0)); +} + +/** All registered channel-feature plugins, in classification order. */ +export function getChannelPlugins(): ChannelFeaturePlugin[] { + return [...plugins]; +} + +/** + * Classify `channel` against an explicit plugin list: the first plugin whose + * `parseBinding` returns non-null wins (the list is assumed already in + * priority order). `null` when none match. The primitive `classifyChannel` + * builds on (passing the mutable registry); exposed for any caller that holds + * a fixed plugin set of its own. + */ +export function classifyChannelWith( + candidates: readonly ChannelFeaturePlugin[], + channel: ChannelClassifyInput, +): ChannelBinding | null { + for (const plugin of candidates) { + const value = plugin.parseBinding(channel); + if (value !== null) { + return { pluginId: plugin.id, value }; + } + } + return null; +} + +/** + * Classify `channel` against the registered plugins: the first plugin (in + * priority order) whose `parseBinding` returns non-null wins. `null` when no + * plugin matches — with the built-in `stream` catch-all plugin registered + * (see `builtins.ts`), this only happens before that registration runs. + */ +export function classifyChannel( + channel: ChannelClassifyInput, +): ChannelBinding | null { + return classifyChannelWith(plugins, channel); +} + +/** The matched plugin's glyph for `channel`, or `null` when nothing matched. */ +export function channelGlyph(channel: ChannelClassifyInput) { + const binding = classifyChannel(channel); + if (!binding) return null; + return ( + plugins.find((plugin) => plugin.id === binding.pluginId)?.glyph ?? null + ); +} + +/** Test-only: reset the registry between test files/cases. */ +export function __resetChannelFeatureRegistryForTests(): void { + plugins = []; +} diff --git a/desktop/src/shared/channel-features/types.ts b/desktop/src/shared/channel-features/types.ts new file mode 100644 index 00000000000..38eed2fd948 --- /dev/null +++ b/desktop/src/shared/channel-features/types.ts @@ -0,0 +1,51 @@ +import type { LucideIcon } from "lucide-react"; +import type { Channel } from "@/shared/api/types"; + +/** + * The minimal channel shape a plugin's `parseBinding` needs to classify a + * channel. Kept narrower than the full `Channel` so call sites that only + * have a partial channel on hand (e.g. `ChatHeader`, which receives + * `channelType`/`visibility` as separate props rather than a `Channel`) can + * classify without constructing one. + */ +export type ChannelClassifyInput = Pick; + +/** + * The result of classifying a channel: which plugin matched, and the value + * its `parseBinding` returned. `T` defaults to `unknown` so call sites that + * don't care about the specific plugin's binding shape (e.g. glyph + * resolution) can use the bare `ChannelBinding` alias. + */ +export interface ChannelBinding { + pluginId: string; + value: T; +} + +/** + * A channel-feature plugin: classifies a channel into a typed binding and + * supplies the glyph shown for channels it matches. Modeled on the existing + * `shared/features/` flag manifest's "typed definition + resolver + gate" + * ergonomics, but for channel-binding plugins instead of preview flags. + * + * This is the seed of the plugin surface proposed in + * https://github.com/block/buzz/issues/3280: today it unifies channel + * classification for the header glyph and the channel-screen content + * dispatch (see `ChatHeader.tsx` / `ChannelScreen.tsx`). A plugin that wants + * to contribute its own tab bar, settings section, or sidebar affordance — + * e.g. hosting an MCP App as a channel tab (block/buzz#3275) — is a natural + * extension of `T` and this interface once a concrete second consumer shows + * up; see PR_DESCRIPTION.md's Follow-ups for the shape that would take. + */ +export interface ChannelFeaturePlugin { + /** Unique id — duplicate registration is a no-op (warn + ignore). */ + id: string; + /** + * Classify `channel` into this plugin's binding shape, or `null` if it + * doesn't match. + */ + parseBinding: (channel: ChannelClassifyInput) => T | null; + /** Glyph shown for channels this plugin matches (header/sidebar/intro). */ + glyph?: LucideIcon; + /** Lower runs first when classifying; ties keep registration order. Default 0. */ + priority?: number; +}