From 99febda256e2bf842814c3251fa0ac8ce67a7d0e Mon Sep 17 00:00:00 2001 From: xuhd Date: Thu, 14 May 2026 17:35:02 +0800 Subject: [PATCH] =?UTF-8?q?feat(skills/notifications):=20=E5=A4=9A?= =?UTF-8?q?=E7=A7=9F=E6=88=B7=E9=98=B6=E6=AE=B5=204=20=E2=80=94=20?= =?UTF-8?q?=E6=B5=8B=E8=AF=95=20+=20=E5=89=8D=E7=AB=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 后端： tests/test_skills_multitenancy.py（12 个用例，全部通过） - SkillStore: 可见性 / 全局重名拒绝 / owner-only update+delete / admin override / ensure_legacy 仅补 NULL owner - 路由助手 _get_editable_skill_or_raise / _get_owned_skill_or_raise 的 owner / admin / 403 / 404 分流 - cascade_skill_unpublished_or_deleted： · unpublish 同时扫两个用户的 agent，剥掉 skill；自己改自己不发 通知，其他人收 agent_skill_removed · admin_takedown 额外给 skill owner 发 skill_taken_down - NotificationStore CRUD + unread_count + 跨用户隔离 - 同步版 apply_automatic_transitions 用户隔离：A 跑 curator 不 动 B 的 skill；curator_log per-user 分桶 前端 — core/notifications/{types,api,hooks,index}.ts - 类型对齐 routers/notifications.py 的 NotificationItem - listNotifications / getUnreadCount / markRead / markAllRead / deleteNotification - useNotifications / useUnreadCount(refetchInterval=30s) / mutation 钩子 前端 — components/workspace/notification-bell.tsx - 顶部铃铛图标 + 未读红点（>99 显示 99+） - 复用 DropdownMenu 作为弹层（项目暂无 Popover 原语） - 列表项：未读蓝点 + 标题 + body + 相对时间，悬停露出删除按钮 - 标记单条已读 / 全部已读 / 删除 - 接入 workspace-nav-menu.tsx，作为 SidebarMenuItem 放在设置入口上方 前端 — Skill 多租户字段 - core/skills/type.ts 加 owner_user_id / published / builtin - core/skills/api.ts 加 setSkillPublished + deleteCustomSkill - core/skills/hooks.ts 加 useSetSkillPublished + useDeleteCustomSkill 前端 — settings/skill-settings-page.tsx - 抽出 SkillRow 子组件，每项渲染 owner badge（内置/我的/来自他人/ 已发布），样式抄自 agent-card.tsx 的 isBuiltin/isOwned/isFromOthers - canManage = isOwned || isAdmin；canTogglePublish 同上但排除内置 - 发布/取消发布按钮（Eye / EyeOff），调用 setSkillPublished - 删除按钮 + 二次确认 Dialog，调用 deleteCustomSkill - 启用开关保持原样 前端 — i18n - types.ts: 新增 NotificationsTranslations + Translations.notifications； settings.skills 加 badgeBuiltin/badgeMine/badgeFromOthers(+Title) / badgePublished(+Title) / publishAction / unpublishAction / publishSuccess / unpublishSuccess / deleteAction / deleteConfirm / deleteSuccess - zh-CN.ts / en-US.ts 同步补齐对应文案 验证 - PYTHONPATH=. uv run pytest tests/test_skills_multitenancy.py -v → 12 passed in 2.08s - 与现有 test_agents_backend / test_skill_evolution_runtime / test_skills_install_upload 并行跑：41 通过 + 1 个 pre-existing 失败 (test_validate_rejects_missing_skill_md 期望 "SKILL.md" 但收到 "Skill archive is empty"，与本期改动无关) - pnpm typecheck：本次新增/修改的文件零新增错误；剩余报错全部为 pre-existing 的 next/link、next/navigation 别名声明缺失、 workspace-nav-menu.tsx 的 SettingsSection 类型不匹配、nextra 缺失、markdown-docx 第三方库等 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../workspace/notification-bell.tsx | 212 +++++++++ .../settings/skill-settings-page.tsx | 220 ++++++++- .../workspace/workspace-nav-menu.tsx | 4 + frontend-web/src/core/i18n/locales/en-US.ts | 23 + frontend-web/src/core/i18n/locales/types.ts | 25 + frontend-web/src/core/i18n/locales/zh-CN.ts | 22 + frontend-web/src/core/notifications/api.ts | 60 +++ frontend-web/src/core/notifications/hooks.ts | 62 +++ frontend-web/src/core/notifications/index.ts | 3 + frontend-web/src/core/notifications/types.ts | 18 + frontend-web/src/core/skills/api.ts | 34 ++ frontend-web/src/core/skills/hooks.ts | 34 +- frontend-web/src/core/skills/type.ts | 5 + .../backend/tests/test_skills_multitenancy.py | 432 ++++++++++++++++++ 14 files changed, 1133 insertions(+), 21 deletions(-) create mode 100644 frontend-web/src/components/workspace/notification-bell.tsx create mode 100644 frontend-web/src/core/notifications/api.ts create mode 100644 frontend-web/src/core/notifications/hooks.ts create mode 100644 frontend-web/src/core/notifications/index.ts create mode 100644 frontend-web/src/core/notifications/types.ts create mode 100644 offline-backend-20260512/backend/tests/test_skills_multitenancy.py diff --git a/frontend-web/src/components/workspace/notification-bell.tsx b/frontend-web/src/components/workspace/notification-bell.tsx new file mode 100644 index 00000000..14310bea --- /dev/null +++ b/frontend-web/src/components/workspace/notification-bell.tsx @@ -0,0 +1,212 @@ +"use client"; + +import { BellIcon, CheckCheckIcon, Loader2, Trash2Icon } from "lucide-react"; +import { useState } from "react"; +import { toast } from "sonner"; + +import { Button } from "@/components/ui/button"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from "@/components/ui/dropdown-menu"; +import { ScrollArea } from "@/components/ui/scroll-area"; +import { + SidebarMenuButton, + useSidebar, +} from "@/components/ui/sidebar"; +import { useI18n } from "@/core/i18n/hooks"; +import { + useDeleteNotification, + useMarkAllNotificationsRead, + useMarkNotificationRead, + useNotifications, + useUnreadCount, +} from "@/core/notifications"; +import type { NotificationItem } from "@/core/notifications"; +import { formatTimeAgo } from "@/core/utils/datetime"; +import { cn } from "@/lib/utils"; + +const BELL_LIMIT = 20; + +export function NotificationBell() { + const { t } = useI18n(); + const { open: isSidebarOpen } = useSidebar(); + const [open, setOpen] = useState(false); + + // Render badge from the lightweight count endpoint so the bell stays cheap + // when nobody opens it. Only when the user actually opens the dropdown do + // we pay for the full list query. + const { data: unreadCount = 0 } = useUnreadCount(); + const { + data: notifications, + isLoading, + isFetching, + } = useNotifications({ limit: BELL_LIMIT }); + const markRead = useMarkNotificationRead(); + const markAllRead = useMarkAllNotificationsRead(); + const removeNotification = useDeleteNotification(); + + const hasUnread = unreadCount > 0; + const items = notifications ?? []; + + async function handleClickItem(notification: NotificationItem) { + if (notification.read) return; + try { + await markRead.mutateAsync(notification.id); + } catch (err) { + toast.error(err instanceof Error ? err.message : String(err)); + } + } + + async function handleMarkAllRead() { + if (!hasUnread) return; + try { + await markAllRead.mutateAsync(); + } catch (err) { + toast.error(err instanceof Error ? err.message : String(err)); + } + } + + async function handleDelete(notification: NotificationItem) { + try { + await removeNotification.mutateAsync(notification.id); + } catch (err) { + toast.error(err instanceof Error ? err.message : String(err)); + } + } + + return ( + + + + {isSidebarOpen ? ( +
+
+ + {hasUnread && ( + + {unreadCount > 99 ? "99+" : unreadCount} + + )} +
+ {t.notifications.title} + {hasUnread && ( + + {t.notifications.unreadShort.replace("{count}", String(unreadCount))} + + )} +
+ ) : ( +
+
+ + {hasUnread && ( + + )} +
+
+ )} + + + +
+
+ + + {t.notifications.title} + + {isFetching && !isLoading && ( + + )} +
+ +
+ + {isLoading ? ( +
+ + {t.common.loading} +
+ ) : items.length === 0 ? ( +
+ {t.notifications.empty} +
+ ) : ( +
    + {items.map((notification) => ( +
  • void handleClickItem(notification)} + > +
    + {!notification.read && ( + + )} +
    +
    +

    + {notification.title || notification.type} +

    + {notification.created_at && ( + + {formatTimeAgo(notification.created_at)} + + )} +
    + {notification.body && ( +

    + {notification.body} +

    + )} +
    + +
    +
    • + ))} +
+ )} + + + + ); +} diff --git a/frontend-web/src/components/workspace/settings/skill-settings-page.tsx b/frontend-web/src/components/workspace/settings/skill-settings-page.tsx index fce90b8d..20c544cb 100644 --- a/frontend-web/src/components/workspace/settings/skill-settings-page.tsx +++ b/frontend-web/src/components/workspace/settings/skill-settings-page.tsx @@ -1,6 +1,17 @@ "use client"; -import { AlertCircle, CheckCircle2, FileArchive, Loader2, SparklesIcon, TrendingUpIcon, Upload } from "lucide-react"; +import { + AlertCircle, + CheckCircle2, + EyeIcon, + EyeOffIcon, + FileArchive, + Loader2, + SparklesIcon, + Trash2Icon, + TrendingUpIcon, + Upload, +} from "lucide-react"; import { useRouter } from "next/navigation"; import { useCallback, useMemo, useRef, useState } from "react"; import { toast } from "sonner"; @@ -10,6 +21,7 @@ import { Button } from "@/components/ui/button"; import { Dialog, DialogContent, + DialogDescription, DialogFooter, DialogHeader, DialogTitle, @@ -31,11 +43,14 @@ import { } from "@/components/ui/item"; import { Switch } from "@/components/ui/switch"; import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs"; +import { getStoredAuth } from "@/core/auth"; import { useI18n } from "@/core/i18n/hooks"; import type { ValidateSkillUploadResponse } from "@/core/skills/api"; import { + useDeleteCustomSkill, useEnableSkill, useInstallSkillUpload, + useSetSkillPublished, useSkills, useValidateSkillUpload, } from "@/core/skills/hooks"; @@ -80,6 +95,11 @@ function SkillSettingsList({ () => skills.filter((skill) => skill.category === filter), [skills, filter], ); + + const storedAuth = useMemo(() => getStoredAuth(), []); + const currentUserId = storedAuth?.user_id ?? null; + const isAdmin = storedAuth?.system_role === "admin"; + const handleCreateSkill = () => { onClose?.(); router.push("/?mode=skill"); @@ -127,25 +147,15 @@ function SkillSettingsList({ )} {filteredSkills.length > 0 && filteredSkills.map((skill) => ( - - - -
{skill.name}
- - - {skill.description} - - - - - enableSkill({ skillName: skill.name, enabled: checked }) - } - /> - - + + enableSkill({ skillName: skill.name, enabled }) + } + /> ))} void; +}) { + const { t } = useI18n(); + const setPublished = useSetSkillPublished(); + const deleteSkill = useDeleteCustomSkill(); + const [confirmDeleteOpen, setConfirmDeleteOpen] = useState(false); + + const isCustom = skill.category === "custom"; + // Built-in flag falls back to "no DB owner" so legacy responses without + // the multitenancy fields still classify public skills correctly. + const isBuiltin = skill.builtin ?? !skill.owner_user_id; + const isOwned = + isCustom && !isBuiltin && !!skill.owner_user_id && skill.owner_user_id === currentUserId; + const isFromOthers = isCustom && !isBuiltin && !isOwned; + // Admin can edit/delete/unpublish anyone's skill; non-admin only their own. + const canManage = isCustom && (isOwned || isAdmin); + const canTogglePublish = canManage && !isBuiltin; + + async function handleTogglePublished(next: boolean) { + try { + await setPublished.mutateAsync({ skillName: skill.name, published: next }); + toast.success( + next + ? t.settings.skills.publishSuccess + : t.settings.skills.unpublishSuccess, + ); + } catch (err) { + toast.error(err instanceof Error ? err.message : String(err)); + } + } + + async function handleDelete() { + try { + await deleteSkill.mutateAsync(skill.name); + toast.success(t.settings.skills.deleteSuccess); + setConfirmDeleteOpen(false); + } catch (err) { + toast.error(err instanceof Error ? err.message : String(err)); + } + } + + return ( + <> + + + +
+ {skill.name} + {isBuiltin && isCustom && ( + + {t.settings.skills.badgeBuiltin} + + )} + {isOwned && ( + + {t.settings.skills.badgeMine} + + )} + {isFromOthers && ( + + {t.settings.skills.badgeFromOthers} + + )} + {isOwned && skill.published && ( + + {t.settings.skills.badgePublished} + + )} +
+ + + {skill.description} + + + + {canTogglePublish && ( + + )} + {canManage && ( + + )} + onToggleEnabled(checked)} + /> + + + + + + {t.settings.skills.deleteAction} + + {t.settings.skills.deleteConfirm.replace("{name}", skill.name)} + + + + + + + + + + ); +} + function EmptySkill({ onCreateSkill }: { onCreateSkill: () => void }) { const { t } = useI18n(); return ( diff --git a/frontend-web/src/components/workspace/workspace-nav-menu.tsx b/frontend-web/src/components/workspace/workspace-nav-menu.tsx index 4e974ede..14ceb474 100644 --- a/frontend-web/src/components/workspace/workspace-nav-menu.tsx +++ b/frontend-web/src/components/workspace/workspace-nav-menu.tsx @@ -19,6 +19,7 @@ import { } from "@/components/ui/sidebar"; import { useI18n } from "@/core/i18n/hooks"; +import { NotificationBell } from "./notification-bell"; import { SettingsDialog } from "./settings"; function NavMenuButtonContent({ @@ -62,6 +63,9 @@ export function WorkspaceNavMenu() { defaultSection={settingsDefaultSection} /> + + + {mounted ? ( diff --git a/frontend-web/src/core/i18n/locales/en-US.ts b/frontend-web/src/core/i18n/locales/en-US.ts index daf01d8a..d690ae6e 100644 --- a/frontend-web/src/core/i18n/locales/en-US.ts +++ b/frontend-web/src/core/i18n/locales/en-US.ts @@ -439,6 +439,20 @@ export const enUS: Translations = { emptyDescription: "Put your agent skill folders under the `/skills/custom` folder under the root folder of CM Assistant.", emptyButton: "Create Your First Skill", + badgeBuiltin: "Built-in", + badgeMine: "Mine", + badgeFromOthers: "From others", + badgeFromOthersTitle: "Owned by user {user_id}", + badgePublished: "Published", + badgePublishedTitle: "Visible to all users", + publishAction: "Publish", + unpublishAction: "Unpublish", + publishSuccess: "Skill published", + unpublishSuccess: "Skill unpublished", + deleteAction: "Delete skill", + deleteConfirm: + "Delete skill \"{name}\"? This cannot be undone and the skill will be removed from any agent that referenced it.", + deleteSuccess: "Skill deleted", installDialog: { title: "Install skill", dropHint: "Drop a .zip or .skill archive here, or", @@ -539,4 +553,13 @@ export const enUS: Translations = { emptyDescription: "Credits and acknowledgements will show here.", }, }, + + notifications: { + title: "Notifications", + bellAria: "Open notifications", + unreadShort: "{count} unread", + markAllRead: "Mark all read", + empty: "No notifications", + deleteAria: "Delete notification", + }, }; diff --git a/frontend-web/src/core/i18n/locales/types.ts b/frontend-web/src/core/i18n/locales/types.ts index 9dafb71d..f5d7a088 100644 --- a/frontend-web/src/core/i18n/locales/types.ts +++ b/frontend-web/src/core/i18n/locales/types.ts @@ -1,5 +1,14 @@ import type { LucideIcon } from "lucide-react"; +export interface NotificationsTranslations { + title: string; + bellAria: string; + unreadShort: string; + markAllRead: string; + empty: string; + deleteAria: string; +} + export interface Translations { // Locale meta locale: { @@ -359,6 +368,19 @@ export interface Translations { emptyTitle: string; emptyDescription: string; emptyButton: string; + badgeBuiltin: string; + badgeMine: string; + badgeFromOthers: string; + badgeFromOthersTitle: string; + badgePublished: string; + badgePublishedTitle: string; + publishAction: string; + unpublishAction: string; + publishSuccess: string; + unpublishSuccess: string; + deleteAction: string; + deleteConfirm: string; + deleteSuccess: string; installDialog: { title: string; dropHint: string; @@ -446,4 +468,7 @@ export interface Translations { emptyDescription: string; }; }; + + // In-app notification inbox (bell in the workspace sidebar) + notifications: NotificationsTranslations; } diff --git a/frontend-web/src/core/i18n/locales/zh-CN.ts b/frontend-web/src/core/i18n/locales/zh-CN.ts index 97895bb3..dbc59dfa 100644 --- a/frontend-web/src/core/i18n/locales/zh-CN.ts +++ b/frontend-web/src/core/i18n/locales/zh-CN.ts @@ -420,6 +420,19 @@ export const zhCN: Translations = { emptyDescription: "将你的 Agent Skill 文件夹放在 CM 助手 根目录下的 `/skills/custom` 文件夹中。", emptyButton: "创建你的第一个技能", + badgeBuiltin: "内置", + badgeMine: "我的", + badgeFromOthers: "来自他人", + badgeFromOthersTitle: "来自用户 {user_id}", + badgePublished: "已发布", + badgePublishedTitle: "该技能已对所有用户可见", + publishAction: "发布", + unpublishAction: "取消发布", + publishSuccess: "已发布", + unpublishSuccess: "已取消发布", + deleteAction: "删除技能", + deleteConfirm: "确认删除技能 “{name}”？该操作不可恢复,且会从引用它的所有专家中自动移除。", + deleteSuccess: "技能已删除", installDialog: { title: "安装技能", dropHint: "拖拽 .zip 或 .skill 文件到此处，或者", @@ -512,4 +525,13 @@ export const zhCN: Translations = { emptyDescription: "相关的致谢信息会展示在这里。", }, }, + + notifications: { + title: "通知", + bellAria: "打开通知", + unreadShort: "{count} 条未读", + markAllRead: "全部已读", + empty: "暂无通知", + deleteAria: "删除通知", + }, }; diff --git a/frontend-web/src/core/notifications/api.ts b/frontend-web/src/core/notifications/api.ts new file mode 100644 index 00000000..0e854bdb --- /dev/null +++ b/frontend-web/src/core/notifications/api.ts @@ -0,0 +1,60 @@ +import { apiFetch } from "@/core/api"; +import { getBackendBaseURL } from "@/core/config"; + +import type { + NotificationItem, + NotificationListResponse, + NotificationUnreadCountResponse, +} from "./types"; + +async function parseDetail(res: Response, fallback: string): Promise { + const data = (await res.json().catch(() => ({}))) as { detail?: unknown }; + const detail = typeof data.detail === "string" && data.detail ? data.detail : null; + return new Error(detail ?? `${fallback} (HTTP ${res.status})`); +} + +export async function listNotifications( + options: { unreadOnly?: boolean; limit?: number } = {}, +): Promise { + const params = new URLSearchParams(); + if (options.unreadOnly) params.set("unread_only", "true"); + if (options.limit) params.set("limit", String(options.limit)); + const query = params.toString(); + const url = `${getBackendBaseURL()}/api/notifications${query ? `?${query}` : ""}`; + const res = await apiFetch(url); + if (!res.ok) throw await parseDetail(res, "Failed to list notifications"); + const data = (await res.json()) as NotificationListResponse; + return data.items; +} + +export async function getUnreadCount(): Promise { + const res = await apiFetch(`${getBackendBaseURL()}/api/notifications/unread_count`); + if (!res.ok) throw await parseDetail(res, "Failed to load unread count"); + const data = (await res.json()) as NotificationUnreadCountResponse; + return data.unread; +} + +export async function markNotificationRead(id: string): Promise { + const res = await apiFetch( + `${getBackendBaseURL()}/api/notifications/${encodeURIComponent(id)}/read`, + { method: "POST" }, + ); + if (!res.ok) throw await parseDetail(res, "Failed to mark notification read"); +} + +export async function markAllNotificationsRead(): Promise { + const res = await apiFetch(`${getBackendBaseURL()}/api/notifications/read_all`, { + method: "POST", + }); + if (!res.ok) throw await parseDetail(res, "Failed to mark all read"); + const data = (await res.json()) as { updated: number }; + return data.updated; +} + +export async function deleteNotification(id: string): Promise { + const res = await apiFetch( + `${getBackendBaseURL()}/api/notifications/${encodeURIComponent(id)}`, + { method: "DELETE" }, + ); + if (!res.ok) throw await parseDetail(res, "Failed to delete notification"); +} diff --git a/frontend-web/src/core/notifications/hooks.ts b/frontend-web/src/core/notifications/hooks.ts new file mode 100644 index 00000000..ab3fbb16 --- /dev/null +++ b/frontend-web/src/core/notifications/hooks.ts @@ -0,0 +1,62 @@ +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; + +import { + deleteNotification, + getUnreadCount, + listNotifications, + markAllNotificationsRead, + markNotificationRead, +} from "./api"; + +const ROOT_KEY = ["notifications"] as const; +const COUNT_KEY = [...ROOT_KEY, "unread-count"] as const; + +export function useNotifications(options: { limit?: number; unreadOnly?: boolean } = {}) { + const { limit = 20, unreadOnly = false } = options; + return useQuery({ + queryKey: [...ROOT_KEY, "list", { limit, unreadOnly }], + queryFn: () => listNotifications({ limit, unreadOnly }), + }); +} + +export function useUnreadCount(refetchInterval = 30000) { + return useQuery({ + queryKey: COUNT_KEY, + queryFn: () => getUnreadCount(), + refetchInterval, + // Keep the badge fresh when the user comes back to the tab without + // waiting for the next interval tick. + refetchOnWindowFocus: true, + }); +} + +function useInvalidateNotifications() { + const queryClient = useQueryClient(); + return () => { + void queryClient.invalidateQueries({ queryKey: ROOT_KEY }); + }; +} + +export function useMarkNotificationRead() { + const invalidate = useInvalidateNotifications(); + return useMutation({ + mutationFn: (id: string) => markNotificationRead(id), + onSuccess: invalidate, + }); +} + +export function useMarkAllNotificationsRead() { + const invalidate = useInvalidateNotifications(); + return useMutation({ + mutationFn: () => markAllNotificationsRead(), + onSuccess: invalidate, + }); +} + +export function useDeleteNotification() { + const invalidate = useInvalidateNotifications(); + return useMutation({ + mutationFn: (id: string) => deleteNotification(id), + onSuccess: invalidate, + }); +} diff --git a/frontend-web/src/core/notifications/index.ts b/frontend-web/src/core/notifications/index.ts new file mode 100644 index 00000000..0733bf1a --- /dev/null +++ b/frontend-web/src/core/notifications/index.ts @@ -0,0 +1,3 @@ +export * from "./api"; +export * from "./hooks"; +export * from "./types"; diff --git a/frontend-web/src/core/notifications/types.ts b/frontend-web/src/core/notifications/types.ts new file mode 100644 index 00000000..ad254950 --- /dev/null +++ b/frontend-web/src/core/notifications/types.ts @@ -0,0 +1,18 @@ +export interface NotificationItem { + id: string; + user_id: string; + type: string; + title: string; + body: string; + payload: Record; + read: boolean; + created_at: string | null; +} + +export interface NotificationListResponse { + items: NotificationItem[]; +} + +export interface NotificationUnreadCountResponse { + unread: number; +} diff --git a/frontend-web/src/core/skills/api.ts b/frontend-web/src/core/skills/api.ts index d2d89fb5..6ba58604 100644 --- a/frontend-web/src/core/skills/api.ts +++ b/frontend-web/src/core/skills/api.ts @@ -110,3 +110,37 @@ export async function installSkillUpload( } return response.json() as Promise; } + +export interface SetSkillPublishedResponse { + name: string; + owner_user_id: string | null; + published: boolean; +} + +export async function setSkillPublished( + skillName: string, + published: boolean, +): Promise { + const response = await apiFetch( + `${getBackendBaseURL()}/api/skills/custom/${encodeURIComponent(skillName)}/published`, + { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ published }), + }, + ); + if (!response.ok) { + throw await parseDetailedError(response, "Failed to update published state"); + } + return response.json() as Promise; +} + +export async function deleteCustomSkill(skillName: string): Promise { + const response = await apiFetch( + `${getBackendBaseURL()}/api/skills/custom/${encodeURIComponent(skillName)}`, + { method: "DELETE" }, + ); + if (!response.ok) { + throw await parseDetailedError(response, "Failed to delete skill"); + } +} diff --git a/frontend-web/src/core/skills/hooks.ts b/frontend-web/src/core/skills/hooks.ts index 5abd7f71..d40aae21 100644 --- a/frontend-web/src/core/skills/hooks.ts +++ b/frontend-web/src/core/skills/hooks.ts @@ -1,6 +1,12 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { enableSkill, installSkillUpload, validateSkillUpload } from "./api"; +import { + deleteCustomSkill, + enableSkill, + installSkillUpload, + setSkillPublished, + validateSkillUpload, +} from "./api"; import { loadSkills } from "."; @@ -46,3 +52,29 @@ export function useInstallSkillUpload() { }, }); } + +export function useSetSkillPublished() { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: ({ + skillName, + published, + }: { + skillName: string; + published: boolean; + }) => setSkillPublished(skillName, published), + onSuccess: () => { + void queryClient.invalidateQueries({ queryKey: ["skills"] }); + }, + }); +} + +export function useDeleteCustomSkill() { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (skillName: string) => deleteCustomSkill(skillName), + onSuccess: () => { + void queryClient.invalidateQueries({ queryKey: ["skills"] }); + }, + }); +} diff --git a/frontend-web/src/core/skills/type.ts b/frontend-web/src/core/skills/type.ts index 13d2f10b..970baf4c 100644 --- a/frontend-web/src/core/skills/type.ts +++ b/frontend-web/src/core/skills/type.ts @@ -4,4 +4,9 @@ export interface Skill { category: string; license: string; enabled: boolean; + // Multitenancy fields — owner_user_id is null for built-in / public skills. + // ``builtin`` mirrors that nullness for explicit branching on the UI side. + owner_user_id?: string | null; + published?: boolean; + builtin?: boolean; } diff --git a/offline-backend-20260512/backend/tests/test_skills_multitenancy.py b/offline-backend-20260512/backend/tests/test_skills_multitenancy.py new file mode 100644 index 00000000..26c18741 --- /dev/null +++ b/offline-backend-20260512/backend/tests/test_skills_multitenancy.py @@ -0,0 +1,432 @@ +"""Multitenancy tests for the Stage 1-3 skills/notifications work. + +Covers: +- ``SkillStore`` per-user visibility, global-name uniqueness, admin override +- ``_get_editable_skill_or_raise`` route helper (owner / admin / 403 / 404 paths) +- ``cascade_skill_unpublished_or_deleted`` removes skills from referencing + agents' ``config.yaml`` and writes notifications via the sender shim +- ``NotificationStore`` CRUD + ``unread_count`` +- Curator per-user isolation — user A's transitions never touch user B's skills +""" + +from __future__ import annotations + +import pytest +import yaml +from fastapi import HTTPException + +from app.gateway.routers.skills import _get_editable_skill_or_raise, _get_owned_skill_or_raise +from deerflow.config import paths as paths_module +from deerflow.persistence.agents import MemoryAgentStore +from deerflow.persistence.notifications import MemoryNotificationStore, set_notification_store +from deerflow.persistence.skills import MemorySkillStore +from deerflow.skills import curator_log +from deerflow.skills.cascade import cascade_skill_unpublished_or_deleted + + +@pytest.fixture +def isolated_paths(tmp_path, monkeypatch): + """Redirect get_paths() at a fresh temp directory for the duration of one test.""" + monkeypatch.setenv("DEER_FLOW_HOME", str(tmp_path)) + monkeypatch.setattr(paths_module, "_paths", None) + yield tmp_path + monkeypatch.setattr(paths_module, "_paths", None) + + +@pytest.fixture +def notification_store(): + """Install a fresh ``MemoryNotificationStore`` as the sender backend. + + Cleared on both setup and teardown so a stale store from a previous test + can never leak in through the global ``_store`` singleton in + ``deerflow.persistence.notifications.sender``. + """ + store = MemoryNotificationStore() + set_notification_store(store) + yield store + set_notification_store(None) + + +# --------------------------------------------------------------------------- +# SkillStore: per-user visibility +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_skill_store_visibility_owner_published_and_builtin() -> None: + store = MemorySkillStore() + + # Builtin (legacy migrated, no human owner) — visible to everyone. + await store.ensure_legacy({"name": "builtin-skill", "owner_user_id": None, "published": True}) + # User A's private skill (not published). + await store.create({"name": "alpha-tool", "owner_user_id": "user-a", "published": False}) + # User B's published skill — visible across users. + await store.create({"name": "beta-tool", "owner_user_id": "user-b", "published": True}) + # User B's private skill — should be invisible to A and C. + await store.create({"name": "gamma-tool", "owner_user_id": "user-b", "published": False}) + + names_for_a = {s["name"] for s in await store.list_visible("user-a")} + assert names_for_a == {"builtin-skill", "alpha-tool", "beta-tool"} + + names_for_c = {s["name"] for s in await store.list_visible("user-c")} + assert names_for_c == {"builtin-skill", "beta-tool"} + + # get_visible respects the same filter. + assert (await store.get_visible("alpha-tool", "user-a")) is not None + assert (await store.get_visible("alpha-tool", "user-c")) is None + assert (await store.get_visible("gamma-tool", "user-a")) is None + + # builtin flag is derived from owner_user_id IS NULL. + builtin = await store.get_any("builtin-skill") + assert builtin is not None and builtin["builtin"] is True + alpha = await store.get_any("alpha-tool") + assert alpha is not None and alpha["builtin"] is False + + +@pytest.mark.asyncio +async def test_skill_store_create_rejects_duplicate_name() -> None: + """Global uniqueness — even an admin can't register a name a normal user owns.""" + store = MemorySkillStore() + await store.create({"name": "shared", "owner_user_id": "user-a"}) + + with pytest.raises(ValueError, match="already exists"): + await store.create({"name": "shared", "owner_user_id": "user-b"}) + + +@pytest.mark.asyncio +async def test_skill_store_only_owner_can_update_or_delete() -> None: + store = MemorySkillStore() + await store.create({"name": "alpha-tool", "owner_user_id": "user-a", "published": False}) + + # Non-owner update returns None and does not mutate. + assert await store.update("alpha-tool", "user-b", {"published": True}) is None + record = await store.get_any("alpha-tool") + assert record is not None and record["published"] is False + + # Owner can update. + updated = await store.update("alpha-tool", "user-a", {"published": True}) + assert updated is not None and updated["published"] is True + + # Non-owner delete fails. + assert await store.delete("alpha-tool", "user-b") is False + assert await store.get_any("alpha-tool") is not None + + # Owner delete succeeds. + assert await store.delete("alpha-tool", "user-a") is True + assert await store.get_any("alpha-tool") is None + + +@pytest.mark.asyncio +async def test_skill_store_admin_override_can_unpublish_and_delete_any_skill() -> None: + """``update_admin`` / ``delete_admin`` bypass ownership — including others' owned skills.""" + store = MemorySkillStore() + await store.create({"name": "alpha-tool", "owner_user_id": "user-a", "published": True}) + + # update_admin can flip published on a skill the admin doesn't own. + updated = await store.update_admin("alpha-tool", {"published": False}) + assert updated is not None and updated["published"] is False + + # delete_admin removes it. + assert await store.delete_admin("alpha-tool") is True + assert await store.get_any("alpha-tool") is None + + # And no-ops on a missing name. + assert await store.update_admin("does-not-exist", {"published": False}) is None + assert await store.delete_admin("does-not-exist") is False + + +@pytest.mark.asyncio +async def test_skill_store_ensure_legacy_patches_null_owner_only() -> None: + store = MemorySkillStore() + # First migration: no owner → claim by admin. + first = await store.ensure_legacy({"name": "legacy-skill", "owner_user_id": "admin-id", "published": True}) + assert first["owner_user_id"] == "admin-id" + + # Second migration: someone else tries to take ownership — refused, original wins. + second = await store.ensure_legacy({"name": "legacy-skill", "owner_user_id": "someone-else"}) + assert second["owner_user_id"] == "admin-id" + + +# --------------------------------------------------------------------------- +# Route helper authorization (mirrors test_agents_backend.py contract) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_get_editable_skill_or_raise_owner_admin_and_403() -> None: + store = MemorySkillStore() + await store.ensure_legacy({"name": "builtin-skill", "owner_user_id": None, "published": True}) + await store.create({"name": "owned", "owner_user_id": "user-a", "published": False}) + await store.create({"name": "others-published", "owner_user_id": "user-b", "published": True}) + + # Owner can edit own skill. + record, mode = await _get_editable_skill_or_raise(store, "owned", "user-a", is_admin=False) + assert mode == "owner" and record["name"] == "owned" + + # Admin can edit a built-in (NULL owner). + record, mode = await _get_editable_skill_or_raise(store, "builtin-skill", "user-a", is_admin=True) + assert mode == "admin" and record["name"] == "builtin-skill" + + # Unlike agents, the spec lets admin take down ANY skill, including + # another user's published one. + record, mode = await _get_editable_skill_or_raise(store, "others-published", "user-a", is_admin=True) + assert mode == "admin" and record["owner_user_id"] == "user-b" + + # Non-admin trying to edit a built-in → 403. + with pytest.raises(HTTPException) as exc: + await _get_editable_skill_or_raise(store, "builtin-skill", "user-a", is_admin=False) + assert exc.value.status_code == 403 + + # Non-admin trying to edit someone else's published skill → 403. + with pytest.raises(HTTPException) as exc: + await _get_editable_skill_or_raise(store, "others-published", "user-a", is_admin=False) + assert exc.value.status_code == 403 + + # Missing skill → 404 regardless of admin. + with pytest.raises(HTTPException) as exc: + await _get_editable_skill_or_raise(store, "missing", "user-a", is_admin=True) + assert exc.value.status_code == 404 + + +@pytest.mark.asyncio +async def test_get_owned_skill_or_raise_distinguishes_404_from_403() -> None: + """Visible-but-not-owned must surface as 403 so the user sees a clear reason.""" + store = MemorySkillStore() + await store.create({"name": "published-from-b", "owner_user_id": "user-b", "published": True}) + + # Owner check on a published-but-not-owned skill returns 403, not 404. + with pytest.raises(HTTPException) as exc: + await _get_owned_skill_or_raise(store, "published-from-b", "user-a") + assert exc.value.status_code == 403 + + # Truly invisible / nonexistent → 404. + with pytest.raises(HTTPException) as exc: + await _get_owned_skill_or_raise(store, "nope", "user-a") + assert exc.value.status_code == 404 + + +# --------------------------------------------------------------------------- +# Cascade: detaches references + writes notifications +# --------------------------------------------------------------------------- + + +def _write_agent_dir(base_dir, *, agent_id: str, skills: list[str]) -> None: + agent_dir = base_dir / "agents" / agent_id + agent_dir.mkdir(parents=True, exist_ok=True) + (agent_dir / "config.yaml").write_text( + yaml.safe_dump( + {"id": agent_id, "name": agent_id, "description": "", "skills": skills}, + default_flow_style=False, + allow_unicode=True, + ), + encoding="utf-8", + ) + + +@pytest.mark.asyncio +async def test_cascade_unpublish_strips_skill_and_notifies_other_owners( + isolated_paths, notification_store +) -> None: + """User A unpublishes ``alpha-tool``. User B's agent loses the skill from its + config and gets an in-app notification. User A's own agent loses the skill + too but no notification (self action).""" + agent_store = MemoryAgentStore() + # User A is the actor; she also owns one of the referencing agents. + await agent_store.create({"id": "agent-of-a", "name": "Agent A", "user_id": "user-a"}) + await agent_store.create({"id": "agent-of-b", "name": "Agent B", "user_id": "user-b"}) + # An agent referencing a different skill — should be untouched. + await agent_store.create({"id": "agent-untouched", "name": "Untouched", "user_id": "user-b"}) + + _write_agent_dir(isolated_paths, agent_id="agent-of-a", skills=["alpha-tool", "other"]) + _write_agent_dir(isolated_paths, agent_id="agent-of-b", skills=["alpha-tool"]) + _write_agent_dir(isolated_paths, agent_id="agent-untouched", skills=["unrelated"]) + + counters = await cascade_skill_unpublished_or_deleted( + "alpha-tool", + actor_user_id="user-a", + operation="unpublish", + skill_owner_user_id="user-a", + agent_store=agent_store, + ) + + assert counters["modified"] == 2 + assert counters["notified"] == 1 # only user-b — not the actor + + # A's agent: skill stripped, no notification for the actor. + a_cfg = yaml.safe_load((isolated_paths / "agents" / "agent-of-a" / "config.yaml").read_text("utf-8")) + assert "alpha-tool" not in a_cfg["skills"] + assert "other" in a_cfg["skills"] + + # B's agent: skill stripped, notification delivered to B. + b_cfg = yaml.safe_load((isolated_paths / "agents" / "agent-of-b" / "config.yaml").read_text("utf-8")) + assert b_cfg["skills"] == [] + + user_b_inbox = await notification_store.list_for_user("user-b") + assert len(user_b_inbox) == 1 + note = user_b_inbox[0] + assert note["type"] == "agent_skill_removed" + assert note["payload"]["skill_name"] == "alpha-tool" + assert note["payload"]["agent_id"] == "agent-of-b" + assert note["payload"]["operation"] == "unpublish" + + # Untouched agent retains its config + no notification fired for it. + untouched_cfg = yaml.safe_load( + (isolated_paths / "agents" / "agent-untouched" / "config.yaml").read_text("utf-8") + ) + assert untouched_cfg["skills"] == ["unrelated"] + assert await notification_store.unread_count("user-a") == 0 + + +@pytest.mark.asyncio +async def test_cascade_admin_takedown_also_notifies_skill_owner( + isolated_paths, notification_store +) -> None: + """Admin force-takedown emits an extra ``skill_taken_down`` notification to + the original skill owner, on top of any per-agent notifications.""" + agent_store = MemoryAgentStore() + await agent_store.create({"id": "agent-of-b", "name": "B", "user_id": "user-b"}) + _write_agent_dir(isolated_paths, agent_id="agent-of-b", skills=["alpha-tool"]) + + await cascade_skill_unpublished_or_deleted( + "alpha-tool", + actor_user_id="admin-id", + operation="admin_takedown", + skill_owner_user_id="user-a", + agent_store=agent_store, + ) + + # B (agent owner) gets the agent-skill-removed notification. + assert (await notification_store.unread_count("user-b")) == 1 + # A (skill owner, not the actor) gets the takedown notification. + a_inbox = await notification_store.list_for_user("user-a") + assert len(a_inbox) == 1 + assert a_inbox[0]["type"] == "skill_taken_down" + assert a_inbox[0]["payload"]["skill_name"] == "alpha-tool" + # Admin (the actor) gets nothing. + assert (await notification_store.unread_count("admin-id")) == 0 + + +# --------------------------------------------------------------------------- +# Notification inbox CRUD +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_notification_store_crud_and_unread_count() -> None: + store = MemoryNotificationStore() + + await store.create({"id": "n1", "user_id": "user-a", "type": "x", "title": "t1"}) + await store.create({"id": "n2", "user_id": "user-a", "type": "x", "title": "t2"}) + await store.create({"id": "n3", "user_id": "user-b", "type": "x", "title": "t3"}) + + assert (await store.unread_count("user-a")) == 2 + assert (await store.unread_count("user-b")) == 1 + + # list_for_user filters by recipient and respects unread_only. + a_items = await store.list_for_user("user-a") + assert {item["id"] for item in a_items} == {"n1", "n2"} + + # Mark one read → unread count drops; second listing reflects state. + assert await store.mark_read("n1", "user-a") is True + assert (await store.unread_count("user-a")) == 1 + unread = await store.list_for_user("user-a", unread_only=True) + assert [item["id"] for item in unread] == ["n2"] + + # Cross-user mark_read is rejected. + assert await store.mark_read("n3", "user-a") is False + assert (await store.unread_count("user-b")) == 1 + + # mark_all_read returns the count of newly-flipped rows. + flipped = await store.mark_all_read("user-a") + assert flipped == 1 + assert (await store.unread_count("user-a")) == 0 + + # Delete behaves the same way — owner-only. + assert await store.delete("n3", "user-a") is False + assert await store.delete("n3", "user-b") is True + assert (await store.list_for_user("user-b")) == [] + + +# --------------------------------------------------------------------------- +# Curator per-user isolation +# --------------------------------------------------------------------------- + + +def test_curator_apply_transitions_only_touches_owned_skills( + isolated_paths, monkeypatch +) -> None: + """User A running curator must not mark / archive user B's skills. + + Reproduces a minimal end-of-life scenario: both users own an idle skill, + but only A's should transition to STALE. + """ + from datetime import UTC, datetime, timedelta + + from deerflow.skills import curator, usage + from deerflow.skills.storage import get_or_new_skill_storage, reset_skill_storage + + # The curator's filter helper normally queries the SkillStore via + # ``get_session_factory``; here we substitute a static mapping so the test + # stays SQL-free. ``apply_automatic_transitions`` is synchronous so we + # cannot wait on the async store from inside it without nested-loop pain. + owned_by = { + "user-a": {"a-skill"}, + "user-b": {"b-skill"}, + } + monkeypatch.setattr(curator, "_list_user_owned_skill_names", lambda uid: owned_by.get(uid, set())) + + # Point the on-disk skills root at the temp dir so the usage helpers write + # to ``{tmp}/skills/.usage.json`` and not into the global singleton state. + reset_skill_storage() + monkeypatch.setattr( + "deerflow.skills.storage._default_skill_storage", + get_or_new_skill_storage(skills_path=str(isolated_paths / "skills")), + raising=False, + ) + + # Stamp both skills as agent-created and old enough to mark stale. + old_iso = (datetime.now(UTC) - timedelta(days=90)).isoformat() + usage._update_entry( + "a-skill", + {"source": usage.SOURCE_AGENT, "created_at": old_iso, "last_used_at": old_iso, "state": usage.STATE_ACTIVE}, + ) + usage._update_entry( + "b-skill", + {"source": usage.SOURCE_AGENT, "created_at": old_iso, "last_used_at": old_iso, "state": usage.STATE_ACTIVE}, + ) + + # Run curator transitions as user-a. + counters = curator.apply_automatic_transitions("user-a", datetime.now(UTC)) + + # Only a-skill counted as checked, only it got transitioned. + assert counters["checked"] == 1 # owned-filter narrows the entry set + assert counters["archived"] == 1 # 90 days idle == archive_after_days + + # The on-disk state reflects the filter. + assert usage.get_entry("a-skill")["state"] == usage.STATE_ARCHIVED + assert usage.get_entry("b-skill")["state"] == usage.STATE_ACTIVE + + reset_skill_storage() + + +def test_curator_log_buckets_records_per_user() -> None: + """Curator log records for one user must not appear in another's view — + guards the dict-of-deques refactor in curator_log.py.""" + curator_log.clear_all() + + curator_log.start_new_run(user_id="user-a") + curator_log.record("info", "alpha event", user_id="user-a") + + curator_log.start_new_run(user_id="user-b") + curator_log.record("info", "beta event", user_id="user-b") + + a_records = curator_log.get_recent(user_id="user-a") + b_records = curator_log.get_recent(user_id="user-b") + + assert [r["message"] for r in a_records] == ["run_started", "alpha event"] + assert [r["message"] for r in b_records] == ["run_started", "beta event"] + + # An unknown user gets an empty buffer rather than a cross-user view. + assert curator_log.get_recent(user_id="user-c") == [] + + curator_log.clear_all()