From 48fccaca329e277f51c4e5b717e4d2170be0fdfa Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 26 Aug 2026 20:49:32 +1000 Subject: [PATCH] fix(desktop): fence pre-join rosters to read-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #6852 review found the newly reachable pre-join open-channel roster was not actually read-only: member rows still surfaced community moderation (Ban/Timeout, from the relay role) and managed-agent lifecycle, access, and activity controls (from the viewer's agent directory), since those authorities don't derive from channel role. Derive one explicit fence, isReadOnlyRoster = channel.isMember !== true, in MembersSidebar and apply it to every non-profile affordance and its supporting action-data fetch: add/search (previously the only gate), community moderation and the restrictions query, role changes, removal, managed-agent bulk lists, and the agent runtimes query. Member cards take a readOnly prop that suppresses the entire row actions menu while identity, presence, status badges, and profile opening stay. Adds a Playwright regression that seeds the strongest pre-join viewer — a community admin who owns a running managed bot inside the unjoined channel — and proves no row menus or bulk controls render before Join, then that moderation and agent lifecycle/access controls appear after. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- .../features/channels/ui/MembersSidebar.tsx | 44 +++++-- .../channels/ui/MembersSidebarMemberCard.tsx | 15 ++- .../ui/useMembersSidebarModeration.ts | 11 +- desktop/tests/e2e/channel-browser.spec.ts | 113 +++++++++++++++++- 4 files changed, 163 insertions(+), 20 deletions(-) diff --git a/desktop/src/features/channels/ui/MembersSidebar.tsx b/desktop/src/features/channels/ui/MembersSidebar.tsx index 2633b5bcef..3564bcdbf0 100644 --- a/desktop/src/features/channels/ui/MembersSidebar.tsx +++ b/desktop/src/features/channels/ui/MembersSidebar.tsx @@ -144,6 +144,13 @@ export function MembersSidebar({ relayUrl, }: MembersSidebarProps) { const channelId = channel?.id ?? null; + // An open channel's roster is reachable before joining, and that pre-join + // view is strictly read-only. Every management affordance — add/search, + // community moderation, managed-agent lifecycle/access, view activity, role + // changes, removal, bulk agent actions — and each supporting action-data + // fetch hangs off this single membership fence. Profile opening stays: it + // is part of reading the roster, not managing it. + const isReadOnlyRoster = channel?.isMember !== true; const queryClient = useQueryClient(); const searchInputRef = React.useRef(null); const [searchQuery, setSearchQuery] = React.useState(""); @@ -239,7 +246,7 @@ export function MembersSidebar({ [rawMembers], ); const canAddMembers = - channel?.isMember === true && + !isReadOnlyRoster && canAddChannelMembers({ channelType: channel?.channelType, visibility: channel?.visibility, @@ -442,17 +449,22 @@ export function MembersSidebar({ ]); const canManageMembers = - selfMember?.role === "owner" || selfMember?.role === "admin"; + !isReadOnlyRoster && + (selfMember?.role === "owner" || selfMember?.role === "admin"); const { - canModerate, + canModerate: canModerateCommunity, isModerationPending, moderationStateByPubkey, onBan, onUnban, onTimeout, onUntimeout, - } = useMembersSidebarModeration(open); + } = useMembersSidebarModeration(open && !isReadOnlyRoster); + // Community moderation authority is relay-role derived and so survives into + // channels the viewer never joined — the pre-join fence still applies before + // it can reach a row. + const canModerate = !isReadOnlyRoster && canModerateCommunity; const isArchived = channel?.archivedAt !== null && channel?.archivedAt !== undefined; @@ -476,18 +488,27 @@ export function MembersSidebar({ [archived, bots, managedAgentByPubkey], ); const managedAgentRuntimesQuery = useManagedAgentRuntimesQuery({ - enabled: open && Boolean(relayUrl) && hasLocalManagedMember, + enabled: + open && !isReadOnlyRoster && Boolean(relayUrl) && hasLocalManagedMember, }); const controllableManagedBots = React.useMemo( () => - bots.flatMap((member) => { - const agent = managedAgentByPubkey.get(normalizePubkey(member.pubkey)); - return agent ? [agent] : []; - }), - [bots, managedAgentByPubkey], + isReadOnlyRoster + ? [] + : bots.flatMap((member) => { + const agent = managedAgentByPubkey.get( + normalizePubkey(member.pubkey), + ); + return agent ? [agent] : []; + }), + [bots, isReadOnlyRoster, managedAgentByPubkey], ); const canRemoveMember = React.useCallback( (member: ChannelMember) => { + if (isReadOnlyRoster) { + return false; + } + return ( (selfMember?.role === "admin" && member.pubkey !== currentPubkey) || (selfMember?.role === "owner" && member.role !== "owner") || @@ -495,7 +516,7 @@ export function MembersSidebar({ member.pubkey === currentPubkey ); }, - [currentPubkey, isMyBot, selfMember], + [currentPubkey, isMyBot, isReadOnlyRoster, selfMember], ); const removableManagedBots = React.useMemo( () => @@ -677,6 +698,7 @@ export function MembersSidebar({ presenceStatus={getAvailability(member.pubkey)} profileAvatarUrl={memberProfile?.avatarUrl ?? null} profileOwnerPubkey={memberProfile?.ownerPubkey} + readOnly={isReadOnlyRoster} viewerIsOwner={viewerIsOwner} /> ); diff --git a/desktop/src/features/channels/ui/MembersSidebarMemberCard.tsx b/desktop/src/features/channels/ui/MembersSidebarMemberCard.tsx index 6cc3da1d9c..bf30c423f5 100644 --- a/desktop/src/features/channels/ui/MembersSidebarMemberCard.tsx +++ b/desktop/src/features/channels/ui/MembersSidebarMemberCard.tsx @@ -76,6 +76,10 @@ type MembersSidebarMemberCardProps = { presenceStatus?: PresenceStatus | null; profileAvatarUrl?: string | null; profileOwnerPubkey?: string | null; + /** Read-only roster row (pre-join open channel): suppresses the entire + * actions menu — moderation, agent lifecycle/access, view activity, + * role/remove — while identity, status badges, and profile opening stay. */ + readOnly: boolean; viewerIsOwner: boolean; }; @@ -145,6 +149,7 @@ export function MembersSidebarMemberCard({ presenceStatus, profileAvatarUrl, profileOwnerPubkey, + readOnly, viewerIsOwner, }: MembersSidebarMemberCardProps) { const roleLabel = formatRoleLabel(member, memberIsBot); @@ -157,9 +162,13 @@ export function MembersSidebarMemberCard({ // owner (whom no moderator can restrict). const canModerateMember = canModerate && !memberIsBot && member.role !== "owner"; - const hasActions = memberIsBot - ? Boolean(managedAgent) || canRemoveMember || canViewActivity - : canRemoveMember || canChangeRole || canModerateMember; + // The read-only fence beats every capability, including ones that don't + // derive from channel role (relay moderation, owning the managed agent). + const hasActions = + !readOnly && + (memberIsBot + ? Boolean(managedAgent) || canRemoveMember || canViewActivity + : canRemoveMember || canChangeRole || canModerateMember); const memberIdentity = (
diff --git a/desktop/src/features/channels/ui/useMembersSidebarModeration.ts b/desktop/src/features/channels/ui/useMembersSidebarModeration.ts index 04c4f1a70b..03e49cd3f6 100644 --- a/desktop/src/features/channels/ui/useMembersSidebarModeration.ts +++ b/desktop/src/features/channels/ui/useMembersSidebarModeration.ts @@ -18,14 +18,17 @@ import type { MemberModerationState } from "./MembersSidebarMemberCard"; /** * Owns community ban/timeout wiring for the members sidebar. Gated by relay * role (owner/admin), independent of the per-channel role — the relay rejects - * the command events otherwise. Restrictions are only fetched while the sidebar - * is open and the caller can moderate. + * the command events otherwise. Restrictions are only fetched while `enabled` + * (sidebar open on a channel the viewer has joined — pre-join rosters are + * read-only) and the caller can moderate. */ -export function useMembersSidebarModeration(open: boolean) { +export function useMembersSidebarModeration(enabled: boolean) { const relayMembershipQuery = useMyRelayMembershipQuery(); const relayRole = relayMembershipQuery.data?.role; const canModerate = relayRole === "owner" || relayRole === "admin"; - const restrictionsQuery = useModerationRestrictionsQuery(open && canModerate); + const restrictionsQuery = useModerationRestrictionsQuery( + enabled && canModerate, + ); const banMutation = useBanMemberMutation(); const unbanMutation = useUnbanMemberMutation(); const timeoutMutation = useTimeoutMemberMutation(); diff --git a/desktop/tests/e2e/channel-browser.spec.ts b/desktop/tests/e2e/channel-browser.spec.ts index 71f6d02e74..5632af2434 100644 --- a/desktop/tests/e2e/channel-browser.spec.ts +++ b/desktop/tests/e2e/channel-browser.spec.ts @@ -1,6 +1,10 @@ import { expect, test, type Page } from "@playwright/test"; -import { installMockBridge, openChannelBrowser } from "../helpers/bridge"; +import { + installMockBridge, + openChannelBrowser, + TEST_IDENTITIES, +} from "../helpers/bridge"; const MOCK_PUBKEY = "deadbeef".repeat(8); const CUSTOM_SECTION = { id: "sec-projects", name: "Projects", order: 0 }; @@ -22,7 +26,25 @@ test.beforeEach(async ({ page }, testInfo) => { page, testInfo.title.includes("failed section create") ? { createChannelErrors: ["Create failed"] } - : undefined, + : testInfo.title.includes("pre-join roster") + ? { + // The strongest-authority pre-join viewer: a community admin who + // also owns a managed bot inside the unjoined channel. Neither + // authority may surface row actions before Join. Membership + // support must be on for the NIP-43 snapshot (and thus the relay + // role) to reach the client at all. + relayRequiresMembership: true, + relayRole: "admin", + managedAgents: [ + { + pubkey: TEST_IDENTITIES.charlie.pubkey, + name: "scout", + status: "running", + channelNames: ["design"], + }, + ], + } + : undefined, ); }); @@ -503,6 +525,93 @@ test("non-member open channel header shows its member count and roster", async ( ).toHaveCount(0); }); +async function openMemberMenu(page: Page, pubkey: string) { + const row = page.getByTestId(`sidebar-member-${pubkey}`); + const trigger = page.getByTestId(`sidebar-member-menu-${pubkey}`); + await row.scrollIntoViewIfNeeded(); + await row.hover(); + // Radix dropdown re-opens are unreliable via pointer after a prior item + // interaction; keyboard open (focus + Enter) is deterministic. + await trigger.focus(); + await trigger.press("Enter"); + await expect(trigger).toHaveAttribute("data-state", "open"); +} + +test("pre-join roster is read-only until the viewer joins", async ({ + page, +}) => { + // Seeded by beforeEach: the viewer is a community admin (relay role) and + // owns the running managed bot "scout" that is a member of #design. + const botPubkey = TEST_IDENTITIES.charlie.pubkey; + const bobPubkey = TEST_IDENTITIES.bob.pubkey; + + await page.goto("/"); + await openChannelBrowser(page); + await page + .getByTestId("browse-channel-design") + .locator("button") + .first() + .click(); + await expect(page.getByTestId("chat-title")).toHaveText("design"); + const joinButton = page.getByRole("button", { name: "Join", exact: true }); + await expect(joinButton).toBeVisible(); + await expect(page.getByTestId("channel-members-trigger")).toHaveAttribute( + "aria-label", + "View channel members (3)", + ); + + await page.getByTestId("channel-members-trigger").click(); + const members = page.getByTestId("members-sidebar"); + await expect(members).toBeVisible(); + + // The roster reads normally — human rows and the owned bot's status badge — + await expect( + members.getByTestId(`sidebar-member-${bobPubkey}`), + ).toBeVisible(); + await expect( + members.getByTestId(`sidebar-member-${botPubkey}`), + ).toBeVisible(); + await expect( + members.getByTestId(`sidebar-managed-agent-status-${botPubkey}`), + ).toBeVisible(); + + // — but neither community-admin moderation nor owned-agent lifecycle may + // surface any row menu or bulk agent control before Join. + await expect( + members.locator('[data-testid^="sidebar-member-menu-"]'), + ).toHaveCount(0); + await expect( + members.getByTestId("members-sidebar-agent-controls"), + ).toHaveCount(0); + + await page.keyboard.press("Escape"); + await expect(members).not.toBeVisible(); + + await joinButton.click(); + await expect(joinButton).toHaveCount(0); + await expect(page.getByTestId("channel-members-trigger")).toHaveAttribute( + "aria-label", + "View channel members (4)", + ); + + // The same authorities light up once the viewer is a member: relay-admin + // moderation on a human row, lifecycle + access management on the owned bot. + await page.getByTestId("channel-members-trigger").click(); + await expect(members).toBeVisible(); + + await openMemberMenu(page, bobPubkey); + await expect(page.getByTestId(`sidebar-ban-${bobPubkey}`)).toBeVisible(); + await page.keyboard.press("Escape"); + + await openMemberMenu(page, botPubkey); + await expect( + page.getByTestId(`sidebar-agent-action-${botPubkey}`), + ).toBeVisible(); + await expect( + page.getByTestId(`sidebar-edit-respond-to-${botPubkey}`), + ).toBeVisible(); +}); + test("joining a channel from browser adds it to the sidebar", async ({ page, }) => {