Address review feedback round 2

Keep context-menu capabilities stable per open session while warming permission queries, make edit requests replayable, and strengthen permission assertions.

Co-authored-by: npub1ux8n2yfs8qfvgd75s7kyhar2mztac355v6vmrz4juc9l3msw4pgstums9e <e18f3511303812c437d487ac4bf46ad897dc46946699b18ab2e60bf8ee0ea851@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: npub1ux8n2yfs8qfvgd75s7kyhar2mztac355v6vmrz4juc9l3msw4pgstums9e <e18f3511303812c437d487ac4bf46ad897dc46946699b18ab2e60bf8ee0ea851@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
npub1ux8n2yfs8qfvgd75s7kyhar2mztac355v6vmrz4juc9l3msw4pgstums9e
2026-07-13 18:12:30 -07:00
parent 4bed899be1
commit 7fac474ea9
7 changed files with 199 additions and 144 deletions
+7 -7
View File
@@ -106,8 +106,8 @@ export function AppShell() {
const [managedChannelId, setManagedChannelId] = React.useState<string | null>(
null,
);
const [openChannelManagementInEditMode, setOpenChannelManagementInEditMode] =
React.useState(false);
const [channelManagementRequest, setChannelManagementRequest] =
React.useState({ edit: false, id: 0 });
const [searchFocusRequest, setSearchFocusRequest] = React.useState(0);
const [browseDialogType, setBrowseDialogType] =
React.useState<BrowseDialogType>(null);
@@ -646,7 +646,10 @@ export function AppShell() {
setManagedChannelId(
typeof channelId === "string" ? channelId : null,
);
setOpenChannelManagementInEditMode(options?.edit === true);
setChannelManagementRequest((request) => ({
edit: options?.edit === true,
id: request.id + 1,
}));
setIsChannelManagementOpen(true);
},
getChannelReadAt,
@@ -912,16 +915,13 @@ export function AppShell() {
channels={channels}
currentPubkey={identityQuery.data?.pubkey}
isChannelManagementOpen={isChannelManagementOpen}
openChannelManagementInEditMode={
openChannelManagementInEditMode
}
channelManagementRequest={channelManagementRequest}
onBrowseChannelJoin={handleBrowseChannelJoin}
onBrowseDialogOpenChange={handleBrowseDialogOpenChange}
onChannelManagementOpenChange={(open) => {
setIsChannelManagementOpen(open);
if (!open) {
setManagedChannelId(null);
setOpenChannelManagementInEditMode(false);
}
}}
onDeleteActiveChannel={() => {
+3 -3
View File
@@ -21,7 +21,7 @@ type AppShellOverlaysProps = {
channels: Channel[];
currentPubkey?: string;
isChannelManagementOpen: boolean;
openChannelManagementInEditMode: boolean;
channelManagementRequest: { edit: boolean; id: number };
onBrowseChannelJoin: (channelId: string) => Promise<void>;
onBrowseDialogOpenChange: (open: boolean) => void;
onChannelManagementOpenChange: (open: boolean) => void;
@@ -35,7 +35,7 @@ export function AppShellOverlays({
channels,
currentPubkey,
isChannelManagementOpen,
openChannelManagementInEditMode,
channelManagementRequest,
onBrowseChannelJoin,
onBrowseDialogOpenChange,
onChannelManagementOpenChange,
@@ -82,7 +82,7 @@ export function AppShellOverlays({
<ChannelManagementSheet
channel={activeChannel}
currentPubkey={currentPubkey}
initiallyEditing={openChannelManagementInEditMode}
managementRequest={channelManagementRequest}
onDeleted={onDeleteActiveChannel}
onOpenChange={onChannelManagementOpenChange}
open={true}
@@ -89,7 +89,7 @@ type ChannelManagementSheetProps = {
channel: Channel | null;
animateSplitEnter?: boolean;
currentPubkey?: string;
initiallyEditing?: boolean;
managementRequest?: { edit: boolean; id: number };
layout?: "overlay" | "split";
onDeleted?: () => void;
onOpenChange: (open: boolean) => void;
@@ -101,7 +101,7 @@ export function ChannelManagementSheet({
animateSplitEnter = false,
channel,
currentPubkey,
initiallyEditing = false,
managementRequest = { edit: false, id: 0 },
layout = "overlay",
onDeleted,
onOpenChange,
@@ -203,10 +203,12 @@ export function ChannelManagementSheet({
// Sync drafts from server only when the sheet opens or the channel changes -
// not on every background refetch, which would clobber in-flight edits.
const syncedForRef = React.useRef<string | null>(null);
const appliedEditRequestRef = React.useRef(0);
React.useEffect(() => {
if (!open) {
// Reset on close so the next open re-syncs from server.
syncedForRef.current = null;
appliedEditRequestRef.current = managementRequest.id;
setIsDeleteDialogOpen(false);
setIsEditDialogOpen(false);
setActiveView("summary");
@@ -217,23 +219,25 @@ export function ChannelManagementSheet({
}
const key = detail.id;
if (syncedForRef.current === key) {
return;
if (syncedForRef.current !== key) {
syncedForRef.current = key;
setNameDraft(detail.name);
setDescriptionDraft(detail.description);
setTopicDraft(detail.topic ?? "");
setPurposeDraft(detail.purpose ?? "");
setIsPrivateDraft(detail.visibility === "private");
setIsEphemeralDraft(detail.ttlSeconds !== null);
setTtlDraft(
detail.ttlSeconds !== null ? formatTtlDuration(detail.ttlSeconds) : "",
);
setActiveView("summary");
}
syncedForRef.current = key;
setNameDraft(detail.name);
setDescriptionDraft(detail.description);
setTopicDraft(detail.topic ?? "");
setPurposeDraft(detail.purpose ?? "");
setIsPrivateDraft(detail.visibility === "private");
setIsEphemeralDraft(detail.ttlSeconds !== null);
setTtlDraft(
detail.ttlSeconds !== null ? formatTtlDuration(detail.ttlSeconds) : "",
);
setIsEditDialogOpen(initiallyEditing);
setActiveView("summary");
}, [detail, initiallyEditing, open]);
if (managementRequest.id !== appliedEditRequestRef.current) {
appliedEditRequestRef.current = managementRequest.id;
setIsEditDialogOpen(managementRequest.edit);
}
}, [detail, managementRequest.edit, managementRequest.id, open]);
if (!channel) {
return null;
@@ -1,3 +1,5 @@
import * as React from "react";
import {
Archive,
Bell,
@@ -32,13 +34,40 @@ import { StatusEmoji } from "@/features/user-status/ui/StatusEmoji";
import type { Channel } from "@/shared/api/types";
import { copyTextToClipboard } from "@/shared/lib/clipboard";
import {
ContextMenu,
ContextMenuContent,
ContextMenuItem,
ContextMenuSeparator,
ContextMenuTrigger,
ContextMenuSub,
ContextMenuSubContent,
ContextMenuSubTrigger,
} from "@/shared/ui/context-menu";
export function ChannelContextMenu({
children,
contentProps,
modal,
}: {
children: React.ReactNode;
contentProps: Omit<
React.ComponentProps<typeof ChannelContextMenuItems>,
"menuOpen"
>;
modal?: boolean;
}) {
const [open, setOpen] = React.useState(false);
return (
<ContextMenu modal={modal} onOpenChange={setOpen}>
<ContextMenuTrigger asChild>{children}</ContextMenuTrigger>
<ContextMenuContent forceMount className={open ? undefined : "hidden"}>
<ChannelContextMenuItems {...contentProps} menuOpen={open} />
</ContextMenuContent>
</ContextMenu>
);
}
function MoveToSectionSubmenu({
channelId,
sections,
@@ -143,6 +172,7 @@ function CopyChannelSubmenu({ channel }: { channel: Channel }) {
export function ChannelContextMenuItems({
channel,
hasUnread,
menuOpen = false,
isMuted,
isStarred,
sections,
@@ -160,6 +190,7 @@ export function ChannelContextMenuItems({
}: {
channel: Channel;
hasUnread: boolean;
menuOpen?: boolean;
isMuted?: boolean;
isStarred?: boolean;
sections?: ChannelSection[];
@@ -203,12 +234,26 @@ export function ChannelContextMenuItems({
currentPubkey,
),
);
const showManagementActions =
channel.channelType !== "dm" &&
channel.archivedAt === null &&
(selfMember?.role === "owner" ||
selfMember?.role === "admin" ||
canManageOwnedAgentChannel);
const canManageChannel =
selfMember?.role === "owner" ||
selfMember?.role === "admin" ||
canManageOwnedAgentChannel;
// Snapshot capabilities for each open session. Permission responses that
// arrive after the menu opens apply on the next open instead of inserting
// immediate actions beneath the user's pointer.
const [showManagementActions, setShowManagementActions] =
React.useState(false);
const menuWasOpenRef = React.useRef(false);
React.useLayoutEffect(() => {
if (menuOpen && !menuWasOpenRef.current) {
setShowManagementActions(
channel.channelType !== "dm" &&
channel.archivedAt === null &&
canManageChannel,
);
}
menuWasOpenRef.current = menuOpen;
}, [canManageChannel, channel.archivedAt, channel.channelType, menuOpen]);
const showStar = Boolean(onStarChannel && onUnstarChannel);
const showReadToggle = hasUnread
? Boolean(onMarkChannelRead)
@@ -41,7 +41,7 @@ import {
SidebarMenuItem,
} from "@/shared/ui/sidebar";
import { ChannelMenuButton } from "@/features/sidebar/ui/SidebarSection";
import { ChannelContextMenuItems } from "@/features/sidebar/ui/ChannelContextMenu";
import { ChannelContextMenu } from "@/features/sidebar/ui/ChannelContextMenu";
import { deferMenuAction } from "@/features/sidebar/ui/sidebarMenuHelpers";
import {
DraggableChannelRow,
@@ -415,24 +415,31 @@ export function ChannelGroupSection({
// AlertDialog. A modal ContextMenu would leave `pointer-events: none`
// stuck on <body> when it closes as the dialog mounts, freezing the
// whole app. Non-modal avoids installing that body guard entirely.
<ContextMenu key={channel.id} modal={false}>
<ContextMenuTrigger asChild>
<SidebarMenuItem className="content-visibility-auto-row">
{draggable ? (
<DraggableChannelRow channelId={channel.id}>
<ChannelMenuButton
channel={channel}
activeWorking={activeWorkingByChannelId?.get(channel.id)}
hasUnread={unreadChannelIds.has(channel.id)}
unreadCount={unreadChannelCounts.get(channel.id) ?? 0}
isMuted={mutedChannelIds?.has(channel.id)}
isActive={
isActiveChannel && selectedChannelId === channel.id
}
onSelectChannel={onSelectChannel}
/>
</DraggableChannelRow>
) : (
<ChannelContextMenu
key={channel.id}
modal={false}
contentProps={{
channel,
hasUnread: unreadChannelIds.has(channel.id),
isMuted: mutedChannelIds?.has(channel.id),
isStarred: starredChannelIds?.has(channel.id),
sections,
assignments,
onMarkChannelRead,
onMarkChannelUnread,
onMuteChannel,
onUnmuteChannel,
onStarChannel,
onUnstarChannel,
onAssignChannel,
onUnassignChannel,
onCreateSectionForChannel,
onLeaveChannel,
}}
>
<SidebarMenuItem className="content-visibility-auto-row">
{draggable ? (
<DraggableChannelRow channelId={channel.id}>
<ChannelMenuButton
channel={channel}
activeWorking={activeWorkingByChannelId?.get(channel.id)}
@@ -444,30 +451,20 @@ export function ChannelGroupSection({
}
onSelectChannel={onSelectChannel}
/>
)}
</SidebarMenuItem>
</ContextMenuTrigger>
<ContextMenuContent>
<ChannelContextMenuItems
channel={channel}
hasUnread={unreadChannelIds.has(channel.id)}
isMuted={mutedChannelIds?.has(channel.id)}
isStarred={starredChannelIds?.has(channel.id)}
sections={sections}
assignments={assignments}
onMarkChannelRead={onMarkChannelRead}
onMarkChannelUnread={onMarkChannelUnread}
onMuteChannel={onMuteChannel}
onUnmuteChannel={onUnmuteChannel}
onStarChannel={onStarChannel}
onUnstarChannel={onUnstarChannel}
onAssignChannel={onAssignChannel}
onUnassignChannel={onUnassignChannel}
onCreateSectionForChannel={onCreateSectionForChannel}
onLeaveChannel={onLeaveChannel}
/>
</ContextMenuContent>
</ContextMenu>
</DraggableChannelRow>
) : (
<ChannelMenuButton
channel={channel}
activeWorking={activeWorkingByChannelId?.get(channel.id)}
hasUnread={unreadChannelIds.has(channel.id)}
unreadCount={unreadChannelCounts.get(channel.id) ?? 0}
isMuted={mutedChannelIds?.has(channel.id)}
isActive={isActiveChannel && selectedChannelId === channel.id}
onSelectChannel={onSelectChannel}
/>
)}
</SidebarMenuItem>
</ChannelContextMenu>
))}
</SidebarMenu>
) : null;
@@ -705,52 +702,49 @@ export function CustomChannelSection({
// modal={false}: see note on the other channel ContextMenu
// above — avoids the pointer-events lockup when Leave
// channel's AlertDialog opens.
<ContextMenu key={channel.id} modal={false}>
<ContextMenuTrigger asChild>
<SidebarMenuItem>
<DraggableChannelRow channelId={channel.id}>
<ChannelMenuButton
channel={channel}
activeWorking={activeWorkingByChannelId?.get(
channel.id,
)}
hasUnread={unreadChannelIds.has(channel.id)}
unreadCount={
unreadChannelCounts.get(channel.id) ?? 0
}
isMuted={mutedChannelIds?.has(channel.id)}
isActive={
isActiveChannel &&
selectedChannelId === channel.id
}
onSelectChannel={onSelectChannel}
/>
</DraggableChannelRow>
</SidebarMenuItem>
</ContextMenuTrigger>
<ContextMenuContent>
<ChannelContextMenuItems
channel={channel}
hasUnread={unreadChannelIds.has(channel.id)}
isMuted={mutedChannelIds?.has(channel.id)}
isStarred={starredChannelIds?.has(channel.id)}
sections={sections}
assignments={assignments}
onMarkChannelRead={onMarkChannelRead}
onMarkChannelUnread={onMarkChannelUnread}
onMuteChannel={onMuteChannel}
onUnmuteChannel={onUnmuteChannel}
onStarChannel={onStarChannel}
onUnstarChannel={onUnstarChannel}
onAssignChannel={onAssignChannel}
onUnassignChannel={onUnassignChannel}
onCreateSectionForChannel={
onCreateSectionForChannel
}
onLeaveChannel={onLeaveChannel}
/>
</ContextMenuContent>
</ContextMenu>
<ChannelContextMenu
key={channel.id}
modal={false}
contentProps={{
channel,
hasUnread: unreadChannelIds.has(channel.id),
isMuted: mutedChannelIds?.has(channel.id),
isStarred: starredChannelIds?.has(channel.id),
sections,
assignments,
onMarkChannelRead,
onMarkChannelUnread,
onMuteChannel,
onUnmuteChannel,
onStarChannel,
onUnstarChannel,
onAssignChannel,
onUnassignChannel,
onCreateSectionForChannel,
onLeaveChannel,
}}
>
<SidebarMenuItem>
<DraggableChannelRow channelId={channel.id}>
<ChannelMenuButton
channel={channel}
activeWorking={activeWorkingByChannelId?.get(
channel.id,
)}
hasUnread={unreadChannelIds.has(channel.id)}
unreadCount={
unreadChannelCounts.get(channel.id) ?? 0
}
isMuted={mutedChannelIds?.has(channel.id)}
isActive={
isActiveChannel &&
selectedChannelId === channel.id
}
onSelectChannel={onSelectChannel}
/>
</DraggableChannelRow>
</SidebarMenuItem>
</ChannelContextMenu>
))}
</SidebarMenu>
) : null}
@@ -9,13 +9,7 @@ import {
X,
} from "lucide-react";
import {
ContextMenu,
ContextMenuContent,
ContextMenuTrigger,
} from "@/shared/ui/context-menu";
import { ChannelContextMenuItems } from "@/features/sidebar/ui/ChannelContextMenu";
import { ChannelContextMenu } from "@/features/sidebar/ui/ChannelContextMenu";
import type { ActiveChannelTurnSummary } from "@/features/agents/activeAgentTurnsStore";
import { formatElapsed } from "@/features/agents/ui/agentSessionUtils";
import { getEphemeralChannelDisplay } from "@/features/channels/lib/ephemeralChannel";
@@ -488,20 +482,20 @@ export function SidebarSection({
// The shared menu always renders copy actions, so every row
// gets a context menu regardless of read/mute availability.
return (
<ContextMenu key={channel.id}>
<ContextMenuTrigger asChild>{menuItem}</ContextMenuTrigger>
<ContextMenuContent>
<ChannelContextMenuItems
channel={channel}
hasUnread={unreadChannelIds.has(channel.id)}
isMuted={mutedChannelIds?.has(channel.id)}
onMarkChannelRead={onMarkChannelRead}
onMarkChannelUnread={onMarkChannelUnread}
onMuteChannel={onMuteChannel}
onUnmuteChannel={onUnmuteChannel}
/>
</ContextMenuContent>
</ContextMenu>
<ChannelContextMenu
key={channel.id}
contentProps={{
channel,
hasUnread: unreadChannelIds.has(channel.id),
isMuted: mutedChannelIds?.has(channel.id),
onMarkChannelRead,
onMarkChannelUnread,
onMuteChannel,
onUnmuteChannel,
}}
>
{menuItem}
</ChannelContextMenu>
);
})}
</SidebarMenu>
+19 -1
View File
@@ -11,6 +11,7 @@ import {
const GENERAL_CHANNEL_ID = "9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50";
const AGENTS_CHANNEL_ID = "94a444a4-c0a3-5966-ab05-530c6ddc2301";
const RANDOM_CHANNEL_ID = "9dae0116-799b-5071-a0a8-fdd30a91a35d";
const MOCK_IDENTITY_PUBKEY = "deadbeef".repeat(8);
// Relay-only agent owned by the mock viewer (see e2eBridge.ts
// OWNED_RELAY_AGENT_PUBKEY). Classified as a bot via mockRelayAgents and
@@ -2365,6 +2366,23 @@ test("channel context menu hides management actions from members and DMs", async
}) => {
await page.goto("/");
await page.getByTestId("channel-random").click();
await expect(page.getByTestId("chat-title")).toHaveText("random");
await page.getByTestId("channel-management-trigger").click();
await expect(page.getByTestId("channel-management-sheet")).toBeVisible();
await expect
.poll(async () => {
const commandLog = await readCommandPayloadLog(page);
return commandLog.some(
({ command, payload }) =>
command === "get_channel_members" &&
(payload as { channelId?: string }).channelId === RANDOM_CHANNEL_ID,
);
})
.toBe(true);
await expect(page.getByTestId("channel-management-edit")).toHaveCount(0);
await page.getByTestId("auxiliary-panel-close").click();
await page.getByTestId("channel-random").click({ button: "right" });
await expect(
page.getByRole("menuitem", { name: "Edit channel" }),
@@ -2395,7 +2413,7 @@ test("channel context menu archives a stream", async ({ page }) => {
await expect(archiveItem).toBeVisible();
await archiveItem.click();
await expect(page.getByTestId("stream-list")).not.toContainText("general");
await expect(page.getByTestId("channel-general")).toHaveCount(0);
await openChannelBrowser(page);
await expect(page.getByTestId("browse-channel-general")).toContainText(
"archived",