diff --git a/.env.example b/.env.example index a62d10f4..fa24122e 100644 --- a/.env.example +++ b/.env.example @@ -39,7 +39,6 @@ SESSION_SECRET= # SESSION_COOKIE_SECURE=false # ── Cache Control ──────────────────────────────────────────── -# These mirror LibreChat's cache env vars. ADMIN_PANEL_* variants # take precedence, falling back to the shared LibreChat equivalents. # Static asset caching (hashed files in /assets/) diff --git a/README.md b/README.md index 508c60d7..3dfa4aae 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # LibreChat Admin Panel -A browser-based management interface for [LibreChat](https://github.com/danny-avila/LibreChat). It connects to the same database as the main application and provides a GUI for tasks that would otherwise require editing `librechat.yaml` directly. +A browser-based management interface for [LibreChat](https://github.com/danny-avila/LibreChat). It communicates with LibreChat exclusively through authenticated backend APIs and provides a GUI for tasks that would otherwise require editing `librechat.yaml` directly. ## Features @@ -36,19 +36,24 @@ docker compose down # stop > Use `http://host.docker.internal:3080` for `VITE_API_BASE_URL` to reach > LibreChat running on the host. +The admin panel is a standalone API client and must not receive MongoDB credentials. +Atomic configuration writes, revision snapshots, history, and rollback are owned by +the LibreChat backend. Deploy the matching backend before, or in the same release +train as, this panel version. + #### Environment variables -| Variable | Required | Default | Description | -| ------------------------------- | ----------------------------------- | -------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------- | -| `PORT` | No | `3000` | Port the admin panel listens on | -| `SESSION_SECRET` | **Yes** (always required in Docker) | Dev fallback only when running `bun dev` locally; no default in the Docker image | Encryption key for sessions (min 32 chars) | -| `VITE_API_BASE_URL` | **Yes** (Docker) | `http://localhost:3080` (local dev only) | LibreChat API server URL; use `http://host.docker.internal:` in Docker | -| `VITE_BASE_PATH` | No | `/` | URL subpath to serve the panel under (e.g., `/adminpanel`). Must match at build time and runtime | -| `API_SERVER_URL` | No | Falls back to `VITE_API_BASE_URL` | Server-side LibreChat API URL when the container reaches LibreChat differently than the browser | -| `ADMIN_SSO_ONLY` | No | `false` | Hide email/password form, SSO only | -| `ADMIN_SSO_ENABLED` | No | `true` | Set `false` to hide the SSO button (and auto-redirect) while keeping email/password login | -| `ADMIN_SESSION_IDLE_TIMEOUT_MS` | No | `1800000` (30 min) | Session idle timeout in ms | -| `SESSION_COOKIE_SECURE` | No | `true` in production, `false` otherwise | Set `false` only for plain-HTTP deployments so the browser keeps the admin session cookie | +| Variable | Required | Default | Description | +| ------------------------------- | ----------------------------------- | -------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | +| `PORT` | No | `3000` | Port the admin panel listens on | +| `SESSION_SECRET` | **Yes** (always required in Docker) | Dev fallback only when running `bun dev` locally; no default in the Docker image | Encryption key for sessions (min 32 chars) | +| `VITE_API_BASE_URL` | **Yes** (Docker) | `http://localhost:3080` (local dev only) | LibreChat API server URL; use `http://host.docker.internal:` in Docker | +| `VITE_BASE_PATH` | No | `/` | URL subpath to serve the panel under (e.g., `/adminpanel`). Must match at build time and runtime | +| `API_SERVER_URL` | No | Falls back to `VITE_API_BASE_URL` | Server-side LibreChat API URL when the container reaches LibreChat differently than the browser | +| `ADMIN_SSO_ONLY` | No | `false` | Hide email/password form, SSO only | +| `ADMIN_SSO_ENABLED` | No | `true` | Set `false` to hide the SSO button (and auto-redirect) while keeping email/password login | +| `ADMIN_SESSION_IDLE_TIMEOUT_MS` | No | `1800000` (30 min) | Session idle timeout in ms | +| `SESSION_COOKIE_SECURE` | No | `true` in production, `false` otherwise | Set `false` only for plain-HTTP deployments so the browser keeps the admin session cookie | For OpenID SSO, the admin panel stores a short-lived PKCE verifier in the `admin-session` cookie before redirecting to LibreChat. If the admin panel is diff --git a/docker-compose.yml b/docker-compose.yml index 4e8b2dfd..7eacb5c3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -6,6 +6,25 @@ services: restart: unless-stopped extra_hosts: - 'host.docker.internal:host-gateway' - env_file: .env environment: - PORT=${PORT:-3000} + - SESSION_SECRET=${SESSION_SECRET} + - VITE_API_BASE_URL=${VITE_API_BASE_URL:-http://host.docker.internal:3080} + - API_SERVER_URL=${API_SERVER_URL:-} + - VITE_BASE_PATH=${VITE_BASE_PATH:-/} + - ADMIN_SSO_ONLY=${ADMIN_SSO_ONLY:-false} + - ADMIN_SSO_ENABLED=${ADMIN_SSO_ENABLED:-true} + - ADMIN_SESSION_IDLE_TIMEOUT_MS=${ADMIN_SESSION_IDLE_TIMEOUT_MS:-1800000} + - SESSION_COOKIE_SECURE=${SESSION_COOKIE_SECURE:-true} + - ADMIN_PANEL_METRICS_SECRET=${ADMIN_PANEL_METRICS_SECRET:-} + - ADMIN_PANEL_CSP_ENFORCE=${ADMIN_PANEL_CSP_ENFORCE:-false} + - STATIC_CACHE_MAX_AGE=${STATIC_CACHE_MAX_AGE:-} + - STATIC_CACHE_S_MAX_AGE=${STATIC_CACHE_S_MAX_AGE:-} + - ADMIN_PANEL_STATIC_CACHE_MAX_AGE=${ADMIN_PANEL_STATIC_CACHE_MAX_AGE:-} + - ADMIN_PANEL_STATIC_CACHE_S_MAX_AGE=${ADMIN_PANEL_STATIC_CACHE_S_MAX_AGE:-} + - INDEX_CACHE_CONTROL=${INDEX_CACHE_CONTROL:-} + - INDEX_PRAGMA=${INDEX_PRAGMA:-} + - INDEX_EXPIRES=${INDEX_EXPIRES:-} + - ADMIN_PANEL_INDEX_CACHE_CONTROL=${ADMIN_PANEL_INDEX_CACHE_CONTROL:-} + - ADMIN_PANEL_INDEX_PRAGMA=${ADMIN_PANEL_INDEX_PRAGMA:-} + - ADMIN_PANEL_INDEX_EXPIRES=${ADMIN_PANEL_INDEX_EXPIRES:-} diff --git a/server.ts b/server.ts index 2c175ecc..66775dc1 100644 --- a/server.ts +++ b/server.ts @@ -29,18 +29,23 @@ if (env.NODE_ENV !== 'development') { } const ONE_DAY = 86400; -const rawMaxAge = Number(env.ADMIN_PANEL_STATIC_CACHE_MAX_AGE ?? env.STATIC_CACHE_MAX_AGE); -const rawSMaxAge = Number(env.ADMIN_PANEL_STATIC_CACHE_S_MAX_AGE ?? env.STATIC_CACHE_S_MAX_AGE); +const firstNonEmpty = (...values: Array): string | undefined => + values.find((value) => typeof value === 'string' && value.trim().length > 0); +const rawMaxAge = Number( + firstNonEmpty(env.ADMIN_PANEL_STATIC_CACHE_MAX_AGE, env.STATIC_CACHE_MAX_AGE), +); +const rawSMaxAge = Number( + firstNonEmpty(env.ADMIN_PANEL_STATIC_CACHE_S_MAX_AGE, env.STATIC_CACHE_S_MAX_AGE), +); const maxAge = Number.isNaN(rawMaxAge) ? ONE_DAY * 2 : rawMaxAge; const sMaxAge = Number.isNaN(rawSMaxAge) ? ONE_DAY : rawSMaxAge; const NO_CACHE: Record = { 'Cache-Control': - env.ADMIN_PANEL_INDEX_CACHE_CONTROL ?? - env.INDEX_CACHE_CONTROL ?? + firstNonEmpty(env.ADMIN_PANEL_INDEX_CACHE_CONTROL, env.INDEX_CACHE_CONTROL) ?? 'no-cache, no-store, must-revalidate', - Pragma: env.ADMIN_PANEL_INDEX_PRAGMA ?? env.INDEX_PRAGMA ?? 'no-cache', - Expires: env.ADMIN_PANEL_INDEX_EXPIRES ?? env.INDEX_EXPIRES ?? '0', + Pragma: firstNonEmpty(env.ADMIN_PANEL_INDEX_PRAGMA, env.INDEX_PRAGMA) ?? 'no-cache', + Expires: firstNonEmpty(env.ADMIN_PANEL_INDEX_EXPIRES, env.INDEX_EXPIRES) ?? '0', }; const LONG_CACHE: Record = { @@ -136,9 +141,10 @@ const server = Bun.serve({ ...(BASE_PATH ? { [`${BASE_PATH}`]: () => Response.redirect(`${BASE_PATH}/`, 302) } : {}), '/*': async (req) => { const url = new URL(req.url); - const metricsPath = BASE_PATH && url.pathname.startsWith(BASE_PATH) - ? url.pathname.slice(BASE_PATH.length) || '/' - : url.pathname; + const metricsPath = + BASE_PATH && url.pathname.startsWith(BASE_PATH) + ? url.pathname.slice(BASE_PATH.length) || '/' + : url.pathname; const res = await withHttpMetrics(req, metricsPath, () => handler.fetch(req)); const patched = new Response(res.body, res); for (const [k, v] of Object.entries(NO_CACHE)) { diff --git a/src/components/Sidebar.tsx b/src/components/Sidebar.tsx index ce99c1e2..46d43ab2 100644 --- a/src/components/Sidebar.tsx +++ b/src/components/Sidebar.tsx @@ -1,5 +1,6 @@ import { useState } from 'react'; import { Icon, Dropdown } from '@clickhouse/click-ui'; +import { useQueryClient } from '@tanstack/react-query'; import { Link, useRouter } from '@tanstack/react-router'; import type * as t from '@/types'; import { useStripAriaExpanded, useCapabilities, useLocalize } from '@/hooks'; @@ -43,6 +44,7 @@ function getUserInitials(user?: { name?: string; email?: string } | null): strin export function Sidebar({ user, collapsed, onToggle }: t.SidebarProps) { const localize = useLocalize(); const router = useRouter(); + const queryClient = useQueryClient(); const { hasCapability } = useCapabilities(); const currentPath = router.state.location.pathname; const [isLoggingOut, setIsLoggingOut] = useState(false); @@ -62,6 +64,7 @@ export function Sidebar({ user, collapsed, onToggle }: t.SidebarProps) { setIsLoggingOut(true); try { const result = await adminLogoutFn(); + queryClient.clear(); if (!result.error && result.redirect) { window.location.href = result.redirect; return; @@ -88,7 +91,11 @@ export function Sidebar({ user, collapsed, onToggle }: t.SidebarProps) { >
- {localize('com_a11y_logo_alt')} + {localize('com_a11y_logo_alt')} {localize('com_auth_title')} diff --git a/src/components/access/AccessPage.tsx b/src/components/access/AccessPage.tsx index 454be704..3a42ecf9 100644 --- a/src/components/access/AccessPage.tsx +++ b/src/components/access/AccessPage.tsx @@ -12,6 +12,7 @@ export function AccessPage({ onTabChange, canReadRoles, canReadGroups, + expectedTenantId, }: t.AccessPageProps) { const localize = useLocalize(); const [createGroupOpen, setCreateGroupOpen] = useState(false); @@ -36,16 +37,30 @@ export function AccessPage({
{activeTab === 'groups' && canReadGroups && ( - setCreateGroupOpen(true)} /> + setCreateGroupOpen(true)} + /> )} {activeTab === 'roles' && canReadRoles && ( - setCreateRoleOpen(true)} /> + setCreateRoleOpen(true)} + /> )}
- setCreateGroupOpen(false)} /> - setCreateRoleOpen(false)} /> + setCreateGroupOpen(false)} + /> + setCreateRoleOpen(false)} + />
); } diff --git a/src/components/access/CreateGroupDialog.tsx b/src/components/access/CreateGroupDialog.tsx index f383777a..2ca1438d 100644 --- a/src/components/access/CreateGroupDialog.tsx +++ b/src/components/access/CreateGroupDialog.tsx @@ -3,12 +3,12 @@ import { Button, Dialog, Tabs } from '@clickhouse/click-ui'; import { useMutation, useQueryClient } from '@tanstack/react-query'; import type { AdminUserSearchResult } from '@librechat/data-schemas'; import type * as t from '@/types'; +import { addGroupMemberFn, createGroupFn, tenantQueryKeys } from '@/server'; import { SelectedMemberList, UserSearchInline } from '@/components/shared'; -import { addGroupMemberFn, createGroupFn } from '@/server'; import { cn, notifySuccess, notifyError } from '@/utils'; import { useLocalize } from '@/hooks'; -export function CreateGroupDialog({ open, onClose }: t.CreateGroupDialogProps) { +export function CreateGroupDialog({ open, expectedTenantId, onClose }: t.CreateGroupDialogProps) { const localize = useLocalize(); const queryClient = useQueryClient(); const [activeTab, setActiveTab] = useState('details'); @@ -29,18 +29,24 @@ export function CreateGroupDialog({ open, onClose }: t.CreateGroupDialogProps) { const mutation = useMutation({ mutationFn: async ({ name: submittedName }: { name: string }) => { const { group } = await createGroupFn({ - data: { name: submittedName, description }, + data: { name: submittedName, description, expectedTenantId }, }); for (const user of selectedUsers) { - await addGroupMemberFn({ data: { groupId: group.id, userId: user.id } }); + await addGroupMemberFn({ + data: { groupId: group.id, userId: user.id, expectedTenantId }, + }); } return { name: submittedName }; }, onSuccess: (data) => { - queryClient.invalidateQueries({ queryKey: ['groups'] }); - queryClient.invalidateQueries({ queryKey: ['groupMembers'] }); - queryClient.invalidateQueries({ queryKey: ['availableScopes'] }); - queryClient.invalidateQueries({ queryKey: ['groupAssignments'] }); + queryClient.invalidateQueries({ queryKey: tenantQueryKeys.groups(expectedTenantId) }); + queryClient.invalidateQueries({ queryKey: tenantQueryKeys.groupMembers(expectedTenantId) }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.availableScopes(expectedTenantId), + }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.groupAssignments(expectedTenantId), + }); notifySuccess(localize('com_toast_group_created', { name: data.name })); resetAndClose(); }, @@ -148,6 +154,7 @@ export function CreateGroupDialog({ open, onClose }: t.CreateGroupDialogProps) { u.id)} onAdd={addUser} + expectedTenantId={expectedTenantId} listboxId="create-group-member-results" disabled={mutation.isPending} /> diff --git a/src/components/access/CreateRoleDialog.tsx b/src/components/access/CreateRoleDialog.tsx index 836f3534..72383856 100644 --- a/src/components/access/CreateRoleDialog.tsx +++ b/src/components/access/CreateRoleDialog.tsx @@ -3,14 +3,14 @@ import { Button, Dialog, Tabs } from '@clickhouse/click-ui'; import { useMutation, useQueryClient } from '@tanstack/react-query'; import type { AdminUserSearchResult } from '@librechat/data-schemas'; import type * as t from '@/types'; -import { addRoleMemberFn, createRoleFn, updateRolePermissionsFn } from '@/server'; +import { addRoleMemberFn, createRoleFn, tenantQueryKeys, updateRolePermissionsFn } from '@/server'; import { SelectedMemberList, UserSearchInline } from '@/components/shared'; import { RolePermissionsPanel } from './RolePermissionsPanel'; import { cn, notifySuccess, notifyError } from '@/utils'; import { defaultPermissions } from '@/constants'; import { useLocalize } from '@/hooks'; -export function CreateRoleDialog({ open, onClose }: t.CreateRoleDialogProps) { +export function CreateRoleDialog({ open, expectedTenantId, onClose }: t.CreateRoleDialogProps) { const localize = useLocalize(); const queryClient = useQueryClient(); const [activeTab, setActiveTab] = useState('details'); @@ -32,18 +32,26 @@ export function CreateRoleDialog({ open, onClose }: t.CreateRoleDialogProps) { const mutation = useMutation({ mutationFn: async ({ name: submittedName }: { name: string }) => { - const { role } = await createRoleFn({ data: { name: submittedName, description } }); - await updateRolePermissionsFn({ data: { id: role.id, permissions } }); + const { role } = await createRoleFn({ + data: { name: submittedName, description, expectedTenantId }, + }); + await updateRolePermissionsFn({ data: { id: role.id, permissions, expectedTenantId } }); for (const user of selectedUsers) { - await addRoleMemberFn({ data: { roleId: role.id, userId: user.id } }); + await addRoleMemberFn({ + data: { roleId: role.id, userId: user.id, expectedTenantId }, + }); } return { name: submittedName }; }, onSuccess: (data) => { - queryClient.invalidateQueries({ queryKey: ['roles'] }); - queryClient.invalidateQueries({ queryKey: ['roleMembers'] }); - queryClient.invalidateQueries({ queryKey: ['availableScopes'] }); - queryClient.invalidateQueries({ queryKey: ['roleAssignments'] }); + queryClient.invalidateQueries({ queryKey: tenantQueryKeys.roles(expectedTenantId) }); + queryClient.invalidateQueries({ queryKey: tenantQueryKeys.roleMembers(expectedTenantId) }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.availableScopes(expectedTenantId), + }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.roleAssignments(expectedTenantId), + }); notifySuccess(localize('com_toast_role_created', { name: data.name })); resetAndClose(); }, @@ -168,6 +176,7 @@ export function CreateRoleDialog({ open, onClose }: t.CreateRoleDialogProps) { u.id)} onAdd={addUser} + expectedTenantId={expectedTenantId} listboxId="create-role-member-results" disabled={mutation.isPending} /> diff --git a/src/components/access/EditGroupDialog.tsx b/src/components/access/EditGroupDialog.tsx index 8c0d6cfb..2450915d 100644 --- a/src/components/access/EditGroupDialog.tsx +++ b/src/components/access/EditGroupDialog.tsx @@ -7,6 +7,7 @@ import { addGroupMemberFn, groupMembersQueryOptions, removeGroupMemberFn, + tenantQueryKeys, updateGroupFn, MEMBERS_PAGE_SIZE, } from '@/server'; @@ -23,7 +24,12 @@ import { useLocalize } from '@/hooks'; type EditGroupTab = 'details' | 'members'; -export function EditGroupDialog({ group, canManage, onClose }: t.EditGroupDialogProps) { +export function EditGroupDialog({ + group, + canManage, + expectedTenantId, + onClose, +}: t.EditGroupDialogProps) { const localize = useLocalize(); const queryClient = useQueryClient(); const [activeTab, setActiveTab] = useState('details'); @@ -36,7 +42,7 @@ export function EditGroupDialog({ group, canManage, onClose }: t.EditGroupDialog const [pendingRemovals, setPendingRemovals] = useState([]); const membersQuery = useQuery({ - ...groupMembersQueryOptions(group?.id ?? '', page), + ...groupMembersQueryOptions(group?.id ?? '', expectedTenantId, page), placeholderData: keepPreviousData, enabled: !!group, }); @@ -76,14 +82,20 @@ export function EditGroupDialog({ group, canManage, onClose }: t.EditGroupDialog mutationFn: async ({ name: submittedName }: { name: string }) => { if (!group) throw new Error(localize('com_access_group_unavailable')); if (detailsDirty) { - await updateGroupFn({ data: { id: group.id, name: submittedName, description } }); + await updateGroupFn({ + data: { id: group.id, name: submittedName, description, expectedTenantId }, + }); } const memberResults = await Promise.allSettled([ ...pendingAdditions.map((user) => - addGroupMemberFn({ data: { groupId: group.id, userId: user.id } }), + addGroupMemberFn({ + data: { groupId: group.id, userId: user.id, expectedTenantId }, + }), ), ...pendingRemovals.map((member) => - removeGroupMemberFn({ data: { groupId: group.id, userId: member.userId } }), + removeGroupMemberFn({ + data: { groupId: group.id, userId: member.userId, expectedTenantId }, + }), ), ]); const failures = memberResults.filter( @@ -98,9 +110,15 @@ export function EditGroupDialog({ group, canManage, onClose }: t.EditGroupDialog return { name: submittedName }; }, onSuccess: (data) => { - queryClient.invalidateQueries({ queryKey: ['groups'] }); - queryClient.invalidateQueries({ queryKey: ['groupAssignments'] }); - queryClient.invalidateQueries({ queryKey: ['groupMembers', group?.id] }); + queryClient.invalidateQueries({ queryKey: tenantQueryKeys.groups(expectedTenantId) }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.groupAssignments(expectedTenantId), + }); + if (group) { + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.groupMemberList(expectedTenantId, group.id), + }); + } notifySuccess(localize('com_toast_group_updated', { name: data.name })); onClose(); }, @@ -203,6 +221,7 @@ export function EditGroupDialog({ group, canManage, onClose }: t.EditGroupDialog diff --git a/src/components/access/EditRoleDialog.tsx b/src/components/access/EditRoleDialog.tsx index 891c1ac2..2022d029 100644 --- a/src/components/access/EditRoleDialog.tsx +++ b/src/components/access/EditRoleDialog.tsx @@ -8,6 +8,7 @@ import { removeRoleMemberFn, roleQueryOptions, roleMembersQueryOptions, + tenantQueryKeys, updateRoleFn, updateRolePermissionsFn, MEMBERS_PAGE_SIZE, @@ -26,7 +27,12 @@ import { useLocalize } from '@/hooks'; type EditRoleTab = 'details' | 'permissions' | 'members'; -export function EditRoleDialog({ role, canManage, onClose }: t.EditRoleDialogProps) { +export function EditRoleDialog({ + role, + canManage, + expectedTenantId, + onClose, +}: t.EditRoleDialogProps) { const localize = useLocalize(); const queryClient = useQueryClient(); const [activeTab, setActiveTab] = useState('details'); @@ -40,7 +46,7 @@ export function EditRoleDialog({ role, canManage, onClose }: t.EditRoleDialogPro const [pendingRemovals, setPendingRemovals] = useState([]); const roleDetail = useQuery({ - ...roleQueryOptions(role?.id ?? ''), + ...roleQueryOptions(role?.id ?? '', expectedTenantId), enabled: !!role, }); @@ -51,7 +57,7 @@ export function EditRoleDialog({ role, canManage, onClose }: t.EditRoleDialogPro }, [roleDetail.data, permissions]); const membersQuery = useQuery({ - ...roleMembersQueryOptions(role?.id ?? '', page), + ...roleMembersQueryOptions(role?.id ?? '', expectedTenantId, page), placeholderData: keepPreviousData, enabled: !!role, }); @@ -100,13 +106,13 @@ export function EditRoleDialog({ role, canManage, onClose }: t.EditRoleDialogPro let roleId = role.id; if (detailsDirty) { const result = await updateRoleFn({ - data: { id: role.id, name: submittedName, description }, + data: { id: role.id, name: submittedName, description, expectedTenantId }, }); roleId = result.role.id; } if (permissionsDirty && permissions) { try { - await updateRolePermissionsFn({ data: { id: roleId, permissions } }); + await updateRolePermissionsFn({ data: { id: roleId, permissions, expectedTenantId } }); } catch (err) { if (detailsDirty) { throw new Error( @@ -119,9 +125,13 @@ export function EditRoleDialog({ role, canManage, onClose }: t.EditRoleDialogPro } } const memberResults = await Promise.allSettled([ - ...pendingAdditions.map((user) => addRoleMemberFn({ data: { roleId, userId: user.id } })), + ...pendingAdditions.map((user) => + addRoleMemberFn({ data: { roleId, userId: user.id, expectedTenantId } }), + ), ...pendingRemovals.map((member) => - removeRoleMemberFn({ data: { roleId, userId: member.userId } }), + removeRoleMemberFn({ + data: { roleId, userId: member.userId, expectedTenantId }, + }), ), ]); const failures = memberResults.filter( @@ -137,15 +147,29 @@ export function EditRoleDialog({ role, canManage, onClose }: t.EditRoleDialogPro return { roleId, name: submittedName }; }, onSuccess: (data) => { - queryClient.invalidateQueries({ queryKey: ['roles'] }); - queryClient.invalidateQueries({ queryKey: ['role', role?.id] }); + queryClient.invalidateQueries({ queryKey: tenantQueryKeys.roles(expectedTenantId) }); + if (role) { + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.roleDetail(expectedTenantId, role.id), + }); + } if (data.roleId !== role?.id) { - queryClient.invalidateQueries({ queryKey: ['role', data.roleId] }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.roleDetail(expectedTenantId, data.roleId), + }); + } + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.roleAssignments(expectedTenantId), + }); + if (role) { + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.roleMemberList(expectedTenantId, role.id), + }); } - queryClient.invalidateQueries({ queryKey: ['roleAssignments'] }); - queryClient.invalidateQueries({ queryKey: ['roleMembers', role?.id] }); if (data.roleId !== role?.id) { - queryClient.invalidateQueries({ queryKey: ['roleMembers', data.roleId] }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.roleMemberList(expectedTenantId, data.roleId), + }); } notifySuccess(localize('com_toast_role_updated', { name: data.name })); onClose(); @@ -292,6 +316,7 @@ export function EditRoleDialog({ role, canManage, onClose }: t.EditRoleDialogPro diff --git a/src/components/access/GroupsTab.tsx b/src/components/access/GroupsTab.tsx index 2819f3c5..c2abd886 100644 --- a/src/components/access/GroupsTab.tsx +++ b/src/components/access/GroupsTab.tsx @@ -10,14 +10,14 @@ import { Pagination, TrashButton, } from '@/components/shared'; -import { deleteGroupFn, groupsQueryOptions, GROUPS_PAGE_SIZE } from '@/server'; +import { deleteGroupFn, groupsQueryOptions, tenantQueryKeys, GROUPS_PAGE_SIZE } from '@/server'; import { cn, notifySuccess, notifyError } from '@/utils'; import { useCapabilities, useLocalize } from '@/hooks'; import { EditGroupDialog } from './EditGroupDialog'; import { SystemCapabilities } from '@/constants'; import { ConfirmDialog } from './ConfirmDialog'; -export function GroupsTab({ onCreateGroup }: t.GroupsTabProps) { +export function GroupsTab({ onCreateGroup, expectedTenantId }: t.GroupsTabProps) { const localize = useLocalize(); const queryClient = useQueryClient(); const { hasCapability } = useCapabilities(); @@ -43,7 +43,7 @@ export function GroupsTab({ onCreateGroup }: t.GroupsTabProps) { }; const { data, isLoading, isError, isFetching } = useQuery({ - ...groupsQueryOptions(page, debouncedSearch), + ...groupsQueryOptions(expectedTenantId, page, debouncedSearch), placeholderData: keepPreviousData, }); @@ -52,12 +52,16 @@ export function GroupsTab({ onCreateGroup }: t.GroupsTabProps) { const totalPages = Math.ceil(total / GROUPS_PAGE_SIZE); const deleteMutation = useMutation({ - mutationFn: (group: AdminGroup) => deleteGroupFn({ data: { id: group.id } }), + mutationFn: (group: AdminGroup) => deleteGroupFn({ data: { id: group.id, expectedTenantId } }), onSuccess: (_data, group) => { - queryClient.invalidateQueries({ queryKey: ['groups'] }); - queryClient.invalidateQueries({ queryKey: ['availableScopes'] }); - queryClient.invalidateQueries({ queryKey: ['groupAssignments'] }); - queryClient.invalidateQueries({ queryKey: ['groupMembers'] }); + queryClient.invalidateQueries({ queryKey: tenantQueryKeys.groups(expectedTenantId) }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.availableScopes(expectedTenantId), + }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.groupAssignments(expectedTenantId), + }); + queryClient.invalidateQueries({ queryKey: tenantQueryKeys.groupMembers(expectedTenantId) }); notifySuccess(localize('com_toast_group_deleted', { name: group.name })); setDeleteTarget(null); if (groups.length === 1) { @@ -143,6 +147,7 @@ export function GroupsTab({ onCreateGroup }: t.GroupsTabProps) { key={editTarget?.id} group={editTarget} canManage={canManage} + expectedTenantId={expectedTenantId} onClose={() => setEditTarget(null)} /> diff --git a/src/components/access/RolesTab.tsx b/src/components/access/RolesTab.tsx index 50d53a17..3b407d5b 100644 --- a/src/components/access/RolesTab.tsx +++ b/src/components/access/RolesTab.tsx @@ -9,14 +9,14 @@ import { EmptyState, TrashButton, } from '@/components/shared'; -import { deleteRoleFn, allRolesQueryOptions, ROLES_PAGE_SIZE } from '@/server'; +import { deleteRoleFn, allRolesQueryOptions, tenantQueryKeys, ROLES_PAGE_SIZE } from '@/server'; import { useCapabilities, useLocalize } from '@/hooks'; import { notifySuccess, notifyError } from '@/utils'; import { EditRoleDialog } from './EditRoleDialog'; import { SystemCapabilities } from '@/constants'; import { ConfirmDialog } from './ConfirmDialog'; -export function RolesTab({ onCreateRole }: t.RolesTabProps) { +export function RolesTab({ onCreateRole, expectedTenantId }: t.RolesTabProps) { const localize = useLocalize(); const queryClient = useQueryClient(); const { hasCapability } = useCapabilities(); @@ -26,7 +26,11 @@ export function RolesTab({ onCreateRole }: t.RolesTabProps) { const [search, setSearch] = useState(''); const [page, setPage] = useState(1); - const { data: allRoles = [], isLoading, isError } = useQuery(allRolesQueryOptions); + const { + data: allRoles = [], + isLoading, + isError, + } = useQuery(allRolesQueryOptions(expectedTenantId)); const filtered = useMemo(() => { if (!search) return allRoles; @@ -48,12 +52,16 @@ export function RolesTab({ onCreateRole }: t.RolesTabProps) { }; const deleteMutation = useMutation({ - mutationFn: (role: t.Role) => deleteRoleFn({ data: { id: role.id } }), + mutationFn: (role: t.Role) => deleteRoleFn({ data: { id: role.id, expectedTenantId } }), onSuccess: (_data, role) => { - queryClient.invalidateQueries({ queryKey: ['roles'] }); - queryClient.invalidateQueries({ queryKey: ['availableScopes'] }); - queryClient.invalidateQueries({ queryKey: ['roleAssignments'] }); - queryClient.invalidateQueries({ queryKey: ['roleMembers'] }); + queryClient.invalidateQueries({ queryKey: tenantQueryKeys.roles(expectedTenantId) }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.availableScopes(expectedTenantId), + }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.roleAssignments(expectedTenantId), + }); + queryClient.invalidateQueries({ queryKey: tenantQueryKeys.roleMembers(expectedTenantId) }); notifySuccess(localize('com_toast_role_deleted', { name: role.name })); setDeleteTarget(null); if (paged.length === 1) { @@ -144,6 +152,7 @@ export function RolesTab({ onCreateRole }: t.RolesTabProps) { key={editTarget?.id} role={editTarget} canManage={canManage} + expectedTenantId={expectedTenantId} onClose={() => setEditTarget(null)} /> diff --git a/src/components/configuration/ConfigPage.tsx b/src/components/configuration/ConfigPage.tsx index 51063b8c..cc7a08d6 100644 --- a/src/components/configuration/ConfigPage.tsx +++ b/src/components/configuration/ConfigPage.tsx @@ -11,14 +11,17 @@ import { bulkSaveProfileValuesFn, getBatchFieldProfilesFn, availableScopesOptions, - resetBaseConfigFieldFn, getResolvedConfigFn, importBaseConfigFn, resetBaseConfigFn, + setBaseConfigActiveFn, baseConfigOptions, + getBaseConfigFn, saveBaseConfigFn, getLangfuseConnectionFn, LANGFUSE_CONNECTION_QUERY_KEY, + configRevisionsOptions, + restoreConfigRevisionFn, } from '@/server'; import { flattenObject, @@ -27,27 +30,44 @@ import { normalizeImportConfig, hasConfigCapability, getTabsWithPermission, + collectSecretFieldPaths, + collectRecordFieldPaths, mapSecretPreviewPaths, secretPathForPreviewPath, stripSecretPreviewValues, notifySuccess, notifyError, } from '@/utils'; -import { useLocalize, useHighlightRef, useActiveSection, useCapabilities } from '@/hooks'; -import { CONFIG_TABS, OTHER_TAB, SECTION_META, HIDDEN_SECTIONS } from './configMeta'; import { applyConfigEdit, + getBlockingConfigReset, + applyConfigReset, buildSavePayload, + detectStaleContainerEdits, + versionedStructuralSharing, mergeIndexedArrayEdits, partitionScopeResetPaths, withLangfuseConfiguredPath, + installIfNewer, } from './utils'; +import { + useLocalize, + useHighlightRef, + useActiveSection, + useCapabilities, + useConfigSession, +} from '@/hooks'; +import { CONFIG_TABS, OTHER_TAB, SECTION_META, HIDDEN_SECTIONS } from './configMeta'; import { validateMcpCrossField } from './sections/McpServersRenderer'; import { ScopeSelector, ScopeTriggerButton } from './ScopeSelector'; -import { StickyActionBar } from '@/components/shared'; import { ConfigTableOfContents } from './ConfigTableOfContents'; import { ResetBaseConfigDialog } from './ResetBaseConfigDialog'; +import { VersionConflictDialog } from './VersionConflictDialog'; +import { refreshBaseConfig } from './queries'; +import { RevisionHistoryDialog } from './RevisionHistoryDialog'; +import { isVersionConflictError } from '@/server/utils/errors'; import { ConfirmSaveDialog } from './ConfirmSaveDialog'; +import { StickyActionBar } from '@/components/shared'; import { ConfigTabContent } from './ConfigTabContent'; import { ImportYamlDialog } from './ImportYamlDialog'; import { ContentToolbar } from './ContentToolbar'; @@ -56,8 +76,14 @@ import { ConfigTabBar } from './ConfigTabBar'; import { InfoBanner } from './InfoBanner'; const routeApi = getRouteApi('/_app/configuration/'); +const appRouteApi = getRouteApi('/_app'); const LAST_SCOPE_KEY = 'config:lastScope'; +const baseConfigQueryKey = (tenantId: string): string[] => [ + ...baseConfigOptions.queryKey, + tenantId, +]; + function collectFieldPaths(fields: t.SchemaField[], prefix = ''): string[] { const paths: string[] = []; for (const field of fields) { @@ -71,30 +97,36 @@ function collectFieldPaths(fields: t.SchemaField[], prefix = ''): string[] { return paths; } -const profileMapOptions = (fieldPaths: string[]) => +const profileMapOptions = (fieldPaths: string[], expectedTenantId?: string) => queryOptions({ - queryKey: ['profileMap', fieldPaths], + queryKey: ['profileMap', expectedTenantId ?? '__pending__', fieldPaths], queryFn: () => - getBatchFieldProfilesFn({ data: { paths: fieldPaths } }).then( - (r: { profileMap: Record }) => r.profileMap, - ), - enabled: fieldPaths.length > 0, + getBatchFieldProfilesFn({ + data: { paths: fieldPaths, expectedTenantId: expectedTenantId! }, + }).then((r: { profileMap: Record }) => r.profileMap), + enabled: fieldPaths.length > 0 && expectedTenantId !== undefined, staleTime: 60_000, }); -function resolvedConfigOptions(scope: t.ScopeSelection) { +function resolvedConfigOptions(scope: t.ScopeSelection, expectedTenantId?: string) { const principalType = scope.type === 'SCOPE' ? scope.scope.principalType : null; const principalId = scope.type === 'SCOPE' ? scope.scope.principalId : null; return queryOptions({ - queryKey: ['resolvedConfig', principalType, principalId] as const, + queryKey: [ + 'resolvedConfig', + expectedTenantId ?? '__pending__', + principalType, + principalId, + ] as const, queryFn: () => getResolvedConfigFn({ data: { principalType: principalType!, principalId: principalId!, + expectedTenantId: expectedTenantId!, }, }), - enabled: principalType != null && principalId != null, + enabled: principalType != null && principalId != null && expectedTenantId !== undefined, staleTime: 60_000, }); } @@ -102,6 +134,7 @@ function resolvedConfigOptions(scope: t.ScopeSelection) { export function ConfigPage({ initialTab, highlightField, initialScope }: t.ConfigPageProps) { const localize = useLocalize(); const queryClient = useQueryClient(); + const { user } = appRouteApi.useRouteContext(); const { hasCapability } = useCapabilities(); const canManageConfig = hasCapability(SystemCapabilities.MANAGE_CONFIGS); const canAssignConfigs = hasCapability(SystemCapabilities.ASSIGN_CONFIGS) || canManageConfig; @@ -120,18 +153,91 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi return perms; }, [schemaTree, hasCapability]); - const { data: baseConfigData } = useQuery(baseConfigOptions); + const [baseTenantScope, setBaseTenantScope] = useState(user?.tenantId ?? ''); + const currentBaseQueryKey = useMemo(() => baseConfigQueryKey(baseTenantScope), [baseTenantScope]); + const { data: baseConfigData } = useQuery({ + ...baseConfigOptions, + queryKey: currentBaseQueryKey, + structuralSharing: versionedStructuralSharing>>( + (value) => value.dbConfigVersion, + (value) => value.effectiveTenantId, + ), + refetchOnMount: 'always', + }); + useEffect(() => { + if ( + baseConfigData?.effectiveTenantId === undefined || + baseConfigData.effectiveTenantId === baseTenantScope + ) { + return; + } + const effectiveTenantId = baseConfigData.effectiveTenantId; + installIfNewer( + queryClient, + baseConfigQueryKey(effectiveTenantId), + baseConfigData, + (value) => value.dbConfigVersion, + (value) => value.effectiveTenantId, + ); + queryClient.removeQueries({ queryKey: currentBaseQueryKey, exact: true }); + setBaseTenantScope(effectiveTenantId); + }, [baseConfigData, baseTenantScope, currentBaseQueryKey, queryClient]); const configValues = baseConfigData?.config ?? null; const dbOverrides = baseConfigData?.dbOverrides; const configuredFromBase = baseConfigData?.configuredFromBase; const schemaDefaults = baseConfigData?.schemaDefaults ?? {}; const flatBaseline = useMemo(() => flattenObject(configValues ?? {}), [configValues]); - const [editedValues, setEditedValues] = useState({}); - const [touchedPaths, setTouchedPaths] = useState>(() => new Set()); + const { + baseline: { + version: frozenBaseVersion, + tenantId: frozenBaseTenantId, + value: frozenFlatBaseline, + }, + adoptBaseline, + draft: editedValues, + setDraft: setEditedValues, + conflictOpen: versionConflictOpen, + setConflictOpen: setVersionConflictOpen, + resolveConflict, + rebasing: rebasingVersion, + discarding: discardingConflict, + } = useConfigSession( + { version: null, tenantId: user?.tenantId ?? '', value: {} }, + {}, + ); + const touchedPaths = useMemo(() => new Set(Object.keys(editedValues)), [editedValues]); const [editSessionId, setEditSessionId] = useState(0); + /** + * Import, Reset, and Restore are only reachable while there are no pending + * field edits (touchedPaths.size === 0 the whole time their dialog is + * open), so the dirty-edit gate below never protects them — a background + * refetch (30s staleTime elapsing, a window-focus refetch, an unrelated + * Langfuse save invalidating the same document) while one of these dialogs + * is open would otherwise silently re-freeze a newer version, and the + * admin's eventual confirm would succeed against that newer version + * instead of the one they were actually looking at when they opened the + * dialog — exactly the stale-administrator overwrite this freezing exists + * to prevent. + */ + const [importOpen, setImportOpen] = useState(false); + const [resetBaseOpen, setResetBaseOpen] = useState(false); + const [historyOpen, setHistoryOpen] = useState(false); + const hasDestructiveDialogOpen = importOpen || resetBaseOpen || historyOpen; + useEffect(() => { + if (baseConfigData && touchedPaths.size === 0 && !hasDestructiveDialogOpen) { + adoptBaseline({ + version: baseConfigData.dbConfigVersion, + tenantId: baseConfigData.effectiveTenantId, + value: flatBaseline, + }); + } + }, [baseConfigData, touchedPaths.size, flatBaseline, hasDestructiveDialogOpen]); + const fieldPaths = useMemo(() => collectFieldPaths(schemaTree), [schemaTree]); const schemaPathSet = useMemo(() => new Set(fieldPaths), [fieldPaths]); + const secretFieldPaths = useMemo(() => collectSecretFieldPaths(schemaTree), [schemaTree]); + const recordFieldPaths = useMemo(() => collectRecordFieldPaths(schemaTree), [schemaTree]); const configuredPaths = useMemo(() => { const paths = new Set(); @@ -205,7 +311,6 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi [navigate], ); - const [importOpen, setImportOpen] = useState(false); const [importSuccess, setImportSuccess] = useState(false); const dismissTimer = useRef | undefined>(undefined); useEffect(() => () => clearTimeout(dismissTimer.current), []); @@ -220,7 +325,6 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi if (Object.keys(editedValues).length > 0) { if (!window.confirm(localize('com_config_unsaved_leave'))) return; setEditedValues({}); - setTouchedPaths(new Set()); } setEditSessionId((id) => id + 1); setConfirmSaveOpen(false); @@ -242,10 +346,11 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi const savedScope = useRef(localStorage.getItem(LAST_SCOPE_KEY) ?? undefined); const scopeToRestore = initialScope ?? savedScope.current; const { data: allScopes } = useQuery({ - ...availableScopesOptions, - enabled: !!scopeToRestore, + ...availableScopesOptions(baseConfigData?.effectiveTenantId ?? ''), + enabled: !!scopeToRestore && baseConfigData?.effectiveTenantId !== undefined, }); const initialScopeApplied = useRef(false); + const activeTenantRef = useRef(user?.tenantId ?? ''); useEffect(() => { if (scopeToRestore && allScopes && !initialScopeApplied.current) { const match = @@ -271,14 +376,18 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi const editingScope: t.ConfigScope | undefined = selectedScope.type === 'SCOPE' ? selectedScope.scope : undefined; - const { data: profileMap = {} } = useQuery(profileMapOptions(fieldPaths)); + const { data: profileMap = {} } = useQuery( + profileMapOptions(fieldPaths, baseConfigData?.effectiveTenantId), + ); const handleProfileChange = useCallback(() => { queryClient.invalidateQueries({ queryKey: ['profileMap'] }); queryClient.invalidateQueries({ queryKey: ['resolvedConfig'] }); }, [queryClient]); - const { data: resolvedData } = useQuery(resolvedConfigOptions(selectedScope)); + const { data: resolvedData } = useQuery( + resolvedConfigOptions(selectedScope, baseConfigData?.effectiveTenantId), + ); const scopeChangedPaths = resolvedData?.changedPaths ?? null; const scopeResolvedValues = resolvedData?.resolvedConfig ?? null; @@ -307,10 +416,22 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi }, [scopeChangedPaths, schemaPathSet]); const { data: langfuseConnection } = useQuery({ - queryKey: LANGFUSE_CONNECTION_QUERY_KEY, - queryFn: () => getLangfuseConnectionFn(), + queryKey: baseConfigData?.effectiveTenantId + ? [...LANGFUSE_CONNECTION_QUERY_KEY, baseConfigData.effectiveTenantId] + : LANGFUSE_CONNECTION_QUERY_KEY, + queryFn: () => + getLangfuseConnectionFn({ + data: { expectedTenantId: baseConfigData!.effectiveTenantId }, + }), + structuralSharing: versionedStructuralSharing< + Awaited> + >( + (value) => value.configVersion, + (value) => value.effectiveTenantId, + ), enabled: !isEditingScope && + baseConfigData?.effectiveTenantId !== undefined && schemaTree.some((section) => section.key === 'langfuse') && sectionPermissions.langfuse?.canEdit === true, retry: false, @@ -413,24 +534,31 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi const handleFieldChange = useCallback( (path: string, value: t.ConfigValue) => { - setTouchedPaths((prev) => { - if (prev.has(path)) return prev; - const next = new Set(prev); - next.add(path); - return next; - }); + if (!isEditingScope && getBlockingConfigReset(editedValues, path)) { + notifyError(localize('com_config_reset_before_edit')); + return; + } setEditedValues((prev) => { - return applyConfigEdit( + const next = applyConfigEdit( prev, path, value, scopeBaseline, baselineIntermediates, baselineContainerPaths, + isEditingScope, ); + return next; }); }, - [scopeBaseline, baselineIntermediates, baselineContainerPaths], + [ + scopeBaseline, + baselineIntermediates, + baselineContainerPaths, + editedValues, + localize, + isEditingScope, + ], ); /** @@ -447,12 +575,6 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi delete next[path]; return next; }); - setTouchedPaths((prev) => { - if (!prev.has(path)) return prev; - const next = new Set(prev); - next.delete(path); - return next; - }); }, []); const isDirty = Object.keys(editedValues).length > 0; @@ -477,15 +599,132 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi const [saving, setSaving] = useState(false); const [saveError, setSaveError] = useState(null); + useEffect(() => { + const effectiveTenantId = baseConfigData?.effectiveTenantId; + if (effectiveTenantId === undefined || effectiveTenantId === activeTenantRef.current) { + return; + } + activeTenantRef.current = effectiveTenantId; + savedScope.current = undefined; + initialScopeApplied.current = true; + localStorage.removeItem(LAST_SCOPE_KEY); + setEditedValues({}); + setEditSessionId((id) => id + 1); + setConfirmSaveOpen(false); + setImportOpen(false); + setResetBaseOpen(false); + setHistoryOpen(false); + setVersionConflictOpen(false); + setScopeSelectorOpen(false); + setSelectedScope({ type: 'BASE' }); + adoptBaseline({ + version: baseConfigData?.dbConfigVersion ?? null, + tenantId: effectiveTenantId, + value: flatBaseline, + }); + queryClient.removeQueries({ queryKey: ['profileMap'] }); + queryClient.removeQueries({ queryKey: ['resolvedConfig'] }); + queryClient.removeQueries({ queryKey: ['availableScopes'] }); + queryClient.removeQueries({ queryKey: ['fieldProfileValues'] }); + queryClient.removeQueries({ queryKey: ['roles'] }); + queryClient.removeQueries({ queryKey: ['groups'] }); + navigate({ search: (prev: Record) => ({ ...prev, scope: undefined }) }); + notifyError(localize('com_config_tenant_changed')); + }, [baseConfigData, flatBaseline, localize, navigate, queryClient]); + const handleDiscard = useCallback(() => { setEditedValues({}); - setTouchedPaths(new Set()); setEditSessionId((id) => id + 1); }, []); + const handleDiscardAfterConflict = useCallback( + () => + resolveConflict('discard', async () => { + const fresh = await refreshBaseConfig(queryClient); + setBaseTenantScope(fresh.effectiveTenantId); + handleDiscard(); + setImportOpen(false); + setResetBaseOpen(false); + setHistoryOpen(false); + adoptBaseline({ + version: fresh.dbConfigVersion, + tenantId: fresh.effectiveTenantId, + value: flattenObject((fresh.config ?? {}) as Record), + }); + }).catch((err: Error) => notifyError(err.message)), + [resolveConflict, adoptBaseline, handleDiscard, queryClient], + ); + + const handleRebaseAfterConflict = useCallback( + () => + resolveConflict('rebase', async () => { + const fresh = await refreshBaseConfig(queryClient); + if (fresh.effectiveTenantId !== frozenBaseTenantId) { + handleDiscard(); + setBaseTenantScope(fresh.effectiveTenantId); + adoptBaseline({ + version: fresh.dbConfigVersion, + tenantId: fresh.effectiveTenantId, + value: flattenObject((fresh.config ?? {}) as Record), + }); + notifyError(localize('com_config_tenant_changed')); + return; + } + + // A numeric array index (endpoints.custom.2, ...) no longer safely + // identifies its original element once the array changed underneath + // the draft, and a whole-array or whole-record add/remove draft was + // computed from the container's old contents — all three risk silently + // overwriting whatever the other admin changed. Drop those specific + // edits instead of trusting them; everything else in the draft still + // replays onto the new baseline. + const newFlatBaseline = flattenObject( + (fresh.config ?? {}) as Record, + ); + const staleContainerPaths = detectStaleContainerEdits( + touchedPaths, + editedValues, + frozenFlatBaseline, + newFlatBaseline, + secretFieldPaths, + ); + if (staleContainerPaths.length > 0) { + setEditedValues((prev) => { + const next = { ...prev }; + for (const path of staleContainerPaths) delete next[path]; + return next; + }); + const count = staleContainerPaths.length; + notifyError( + count === 1 + ? localize('com_config_version_conflict_indexed_dropped', { count }) + : localize('com_config_version_conflict_indexed_dropped_plural', { count }), + ); + } + + adoptBaseline({ + version: fresh.dbConfigVersion, + tenantId: fresh.effectiveTenantId, + value: newFlatBaseline, + }); + setEditSessionId((id) => id + 1); + }).catch((err: Error) => notifyError(err.message)), + [ + resolveConflict, + adoptBaseline, + queryClient, + touchedPaths, + editedValues, + frozenFlatBaseline, + frozenBaseTenantId, + secretFieldPaths, + localize, + handleDiscard, + ], + ); + const clearEdits = useCallback(() => { setEditedValues({}); - setTouchedPaths(new Set()); setEditSessionId((id) => id + 1); setConfirmSaveOpen(false); setSaving(false); @@ -493,34 +732,97 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi notifySuccess(localize('com_config_saved')); }, [localize]); - const invalidateAndResetBase = useCallback(() => { - queryClient.invalidateQueries({ queryKey: ['baseConfig'] }); + // Awaited (not fire-and-forget) so `baseConfigData` reflects the new + // version by the time `clearEdits` drops touchedPaths to 0 — otherwise the + // frozen-version re-sync effect fires immediately against the still-stale + // cached data, and an admin who starts a new edit before the background + // refetch lands would freeze on that stale version, 409ing on the next save. + const invalidateAndResetBase = useCallback(async () => { + await Promise.all([ + queryClient.invalidateQueries({ queryKey: ['baseConfig'] }), + queryClient.invalidateQueries({ queryKey: ['configRevisions'] }), + ]); clearEdits(); }, [queryClient, clearEdits]); - const invalidateAndResetScope = useCallback(() => { - queryClient.invalidateQueries({ queryKey: ['resolvedConfig'] }); - queryClient.invalidateQueries({ queryKey: ['profileMap'] }); - queryClient.invalidateQueries({ queryKey: ['availableScopes'] }); + const invalidateAndResetScope = useCallback(async () => { + await Promise.all([ + queryClient.invalidateQueries({ queryKey: ['resolvedConfig'] }), + queryClient.invalidateQueries({ queryKey: ['profileMap'] }), + queryClient.invalidateQueries({ queryKey: ['availableScopes'] }), + ]); clearEdits(); }, [queryClient, clearEdits]); const importMutation = useMutation({ - mutationFn: (config: Record) => importBaseConfigFn({ data: { config } }), - onError: (err: Error) => notifyError(err.message), + mutationFn: (config: Record) => + importBaseConfigFn({ + data: { + config, + expectedVersion: frozenBaseVersion, + expectedTenantId: frozenBaseTenantId, + }, + }), + onError: (err: Error) => { + notifyError(err.message); + // The import never landed — any in-progress edit draft is still valid + // and must not be silently discarded (see VersionConflictDialog). + if (isVersionConflictError(err)) { + setVersionConflictOpen(true); + } + }, onSuccess: invalidateAndResetBase, }); - const [resetBaseOpen, setResetBaseOpen] = useState(false); const [resettingBase, setResettingBase] = useState(false); + const [activatingBase, setActivatingBase] = useState(false); const [resetBaseError, setResetBaseError] = useState(null); + const [restoringRevision, setRestoringRevision] = useState(false); + const [restoreError, setRestoreError] = useState(null); + + const revisionsQuery = useQuery({ + ...configRevisionsOptions(user?.id ?? '', baseConfigData?.effectiveTenantId), + enabled: historyOpen && canManageConfig && !isEditingScope, + }); + + const handleActivateBaseConfig = useCallback(async () => { + if (activatingBase || isDirty) return; + setActivatingBase(true); + try { + await setBaseConfigActiveFn({ + data: { + isActive: true, + expectedVersion: frozenBaseVersion, + expectedTenantId: frozenBaseTenantId, + }, + }); + await Promise.all([ + queryClient.invalidateQueries({ queryKey: ['baseConfig'] }), + queryClient.invalidateQueries({ queryKey: ['configRevisions'] }), + ]); + notifySuccess(localize('com_config_reactivate_success')); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + notifyError(message); + if (isVersionConflictError(err)) { + setVersionConflictOpen(true); + } + } finally { + setActivatingBase(false); + } + }, [activatingBase, frozenBaseTenantId, frozenBaseVersion, isDirty, localize, queryClient]); const handleResetBaseConfig = useCallback(async () => { if (resettingBase) return; setResettingBase(true); setResetBaseError(null); try { - await resetBaseConfigFn(); + await resetBaseConfigFn({ + data: { + expectedVersion: frozenBaseVersion, + expectedTenantId: frozenBaseTenantId, + }, + }); /** resolvedConfig holds each scope's own overrides (not a base merge), so a * base reset doesn't make it stale on its own — but base-derived data * (schemaDefaults, base values used for MCP inheritance) feeds scope mode, @@ -528,9 +830,9 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi await Promise.all([ queryClient.invalidateQueries({ queryKey: ['baseConfig'] }), queryClient.invalidateQueries({ queryKey: ['resolvedConfig'] }), + queryClient.invalidateQueries({ queryKey: ['configRevisions'] }), ]); setEditedValues({}); - setTouchedPaths(new Set()); setEditSessionId((id) => id + 1); setResettingBase(false); setResetBaseOpen(false); @@ -540,24 +842,69 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi setResettingBase(false); setResetBaseError(message); notifyError(message); + // The reset never landed — any in-progress edit draft is still valid + // and must not be silently discarded (see VersionConflictDialog). + if (isVersionConflictError(err)) { + setVersionConflictOpen(true); + } } - }, [resettingBase, queryClient, localize]); + }, [resettingBase, queryClient, localize, frozenBaseVersion, frozenBaseTenantId]); + + const handleRestoreRevision = useCallback( + async (id: string) => { + if (restoringRevision) return; + setRestoringRevision(true); + setRestoreError(null); + try { + await restoreConfigRevisionFn({ + data: { + id, + expectedVersion: frozenBaseVersion, + expectedTenantId: frozenBaseTenantId, + }, + }); + await Promise.all([ + queryClient.invalidateQueries({ queryKey: ['baseConfig'] }), + queryClient.invalidateQueries({ queryKey: ['resolvedConfig'] }), + queryClient.invalidateQueries({ queryKey: ['configRevisions'] }), + ]); + setEditedValues({}); + setEditSessionId((n) => n + 1); + setRestoringRevision(false); + setHistoryOpen(false); + notifySuccess(localize('com_config_revision_success')); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + setRestoringRevision(false); + setRestoreError(message); + notifyError(message); + // The restore never landed — any in-progress edit draft is still + // valid and must not be silently discarded (see VersionConflictDialog). + if (isVersionConflictError(err)) { + setVersionConflictOpen(true); + } + } + }, + [restoringRevision, queryClient, localize, frozenBaseVersion, frozenBaseTenantId], + ); const handleResetField = useCallback((fieldPath: string) => { startTransition(() => { - setTouchedPaths((prev) => { - if (prev.has(fieldPath)) return prev; - const next = new Set(prev); - next.add(fieldPath); + setEditedValues((prev) => { + const next = applyConfigReset(prev, fieldPath); return next; }); - setEditedValues((prev) => ({ ...prev, [fieldPath]: undefined })); }); }, []); const handleConfirmSave = useCallback(async () => { if (saving) return; - const { touched, saves, resets } = buildSavePayload(touchedPaths, editedValues, schemaPathSet); + const { touched, saves, resets } = buildSavePayload( + touchedPaths, + editedValues, + schemaPathSet, + recordFieldPaths, + ); if (touched.length === 0) return; /** Per-leaf saves can land an MCP entry in a transport state whose required siblings are missing (e.g. type=stdio with no command/args). Server-side per-field validation only sees one path at a time, so do the cross-field check here against the merged effective entry before any PATCH fires. Use baseActiveConfigValues so scope-mode edits validate against the scope-resolved baseline (where prior scope overrides supply some required fields) instead of the base config alone. */ @@ -605,75 +952,89 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi setSaveError(null); try { - /** Resets must land before saves so a delete-then-recreate at the same path (e.g. MCP entry replaced with different fields) wipes stale fields first and the new leaf PATCHes don't race against the DELETE. */ - if (resets.length > 0) { - const resetPromises = isEditingScope - ? (() => { - const { resetPaths, tombstonePaths } = partitionScopeResetPaths( - resets, - inheritedMcpKeys, - ); - return [ - ...resetPaths.map((fieldPath) => - removeFieldProfileValueFn({ - data: { - fieldPath, - principalType: editingScope!.principalType, - principalId: editingScope!.principalId, - }, - }), - ), - ...tombstonePaths.map((fieldPath) => - tombstoneFieldProfileValueFn({ - data: { - fieldPath, - principalType: editingScope!.principalType, - principalId: editingScope!.principalId, - }, - }), - ), - ]; - })() - : resets.map((fieldPath) => resetBaseConfigFieldFn({ data: { fieldPath } })); - if (resetPromises.length > 0) { - await Promise.all(resetPromises); + /** Resets must land before saves so a delete-then-recreate at the same path (e.g. MCP entry replaced with different fields) wipes stale fields first and the new leaf PATCHes don't race against the DELETE. Base mode sends both in one server call so a single snapshot is taken before either DELETE or PATCH. */ + if (isEditingScope) { + if (resets.length > 0) { + const { resetPaths, tombstonePaths } = partitionScopeResetPaths(resets, inheritedMcpKeys); + const resetPromises = [ + ...resetPaths.map((fieldPath) => + removeFieldProfileValueFn({ + data: { + fieldPath, + principalType: editingScope!.principalType, + principalId: editingScope!.principalId, + expectedTenantId: frozenBaseTenantId, + }, + }), + ), + ...tombstonePaths.map((fieldPath) => + tombstoneFieldProfileValueFn({ + data: { + fieldPath, + principalType: editingScope!.principalType, + principalId: editingScope!.principalId, + expectedTenantId: frozenBaseTenantId, + }, + }), + ), + ]; + if (resetPromises.length > 0) { + await Promise.all(resetPromises); + } } - } - - if (saves.length > 0) { - if (isEditingScope) { + if (saves.length > 0) { await bulkSaveProfileValuesFn({ data: { principalType: editingScope!.principalType, principalId: editingScope!.principalId, + expectedTenantId: frozenBaseTenantId, entries: saves, }, }); - } else { - await saveBaseConfigFn({ data: { entries: saves } }); } + } else { + await saveBaseConfigFn({ + data: { + entries: saves, + resetPaths: resets, + expectedVersion: frozenBaseVersion, + expectedTenantId: frozenBaseTenantId, + }, + }); } if (isEditingScope) { - invalidateAndResetScope(); + await invalidateAndResetScope(); } else { - invalidateAndResetBase(); + await invalidateAndResetBase(); } } catch (err) { const message = err instanceof Error ? err.message : String(err); setSaving(false); setSaveError(message); notifyError(message); + /** A version conflict can never succeed by retrying with the same frozen + * version, but a long edit session's draft must not be silently thrown + * away the moment CAS detects concurrent work — hand the admin an + * explicit choice instead (VersionConflictDialog): rebase onto the + * latest version and keep editing, or discard and start fresh. */ + if (!isEditingScope && isVersionConflictError(err)) { + setConfirmSaveOpen(false); + setVersionConflictOpen(true); + } } }, [ touchedPaths, editedValues, schemaPathSet, + recordFieldPaths, saving, isEditingScope, baseActiveConfigValues, configValues, baseConfigData, + frozenBaseVersion, + frozenBaseTenantId, localize, editingScope, invalidateAndResetScope, @@ -683,10 +1044,14 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi const serializedEditedValues = useMemo(() => { const result: t.FlatConfigMap = {}; for (const [k, v] of Object.entries(editedValues)) { - result[k] = stripSecretPreviewValues(deepSerializeKVPairs(v), k, schemaPathSet); + result[k] = stripSecretPreviewValues( + deepSerializeKVPairs(v, k, recordFieldPaths), + k, + schemaPathSet, + ); } return result; - }, [editedValues, schemaPathSet]); + }, [editedValues, schemaPathSet, recordFieldPaths]); const originalValuesForDialog = useMemo(() => { const baseline = isEditingScope ? scopeBaseline : flatBaseline; @@ -739,6 +1104,7 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi data: { principalType: scope.principalType, principalId: scope.principalId, + expectedTenantId: frozenBaseTenantId, entries, }, }); @@ -754,7 +1120,7 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi }), ); }, - [queryClient, localize, showImportSuccess, schemaPathSet], + [queryClient, localize, showImportSuccess, schemaPathSet, frozenBaseTenantId], ); const handleImport = useCallback( @@ -952,10 +1318,36 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi return undefined; })(); + const historyTitle = (() => { + if (!canManageConfig) { + return localize('com_cap_no_permission', { cap: SystemCapabilities.MANAGE_CONFIGS }); + } + if (isDirty) return localize('com_config_revision_dirty'); + return undefined; + })(); + return (
{banner &&
{banner}
} + {!isEditingScope && baseConfigData?.dbIsActive === false && ( +
+ {localize('com_config_base_inactive')} + +
+ )} { + setRestoreError(null); + setHistoryOpen(true); + }} showScope={permissions.canView} scopeSelection={selectedScope} onScopeClick={() => setScopeSelectorOpen(true)} @@ -1024,6 +1423,7 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi schemaDefaults={schemaDefaults} showConfiguredOnly={showConfiguredOnly} isEditingScope={isEditingScope} + effectiveTenantId={baseConfigData?.effectiveTenantId} baseRecordKeys={baseRecordKeys} onValidationError={(message) => notifyError(message)} editSessionId={editSessionId} @@ -1060,8 +1460,17 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi onCancel={() => setConfirmSaveOpen(false)} /> + + setImportOpen(false)} onImport={handleImport} onImportAsProfile={handleImportAsProfile} @@ -1087,6 +1497,23 @@ export function ConfigPage({ initialTab, highlightField, initialScope }: t.Confi setResetBaseError(null); }} /> + + { + if (restoringRevision) return; + setHistoryOpen(false); + setRestoreError(null); + }} + />
); } @@ -1100,6 +1527,10 @@ function HeaderActions({ resetDisabled, resetTitle, onResetClick, + showHistory, + historyDisabled, + historyTitle, + onHistoryClick, showScope, scopeSelection, onScopeClick, @@ -1112,6 +1543,10 @@ function HeaderActions({ resetDisabled: boolean; resetTitle?: string; onResetClick: () => void; + showHistory: boolean; + historyDisabled: boolean; + historyTitle?: string; + onHistoryClick: () => void; showScope: boolean; scopeSelection: t.ScopeSelection; onScopeClick: () => void; @@ -1155,6 +1590,21 @@ function HeaderActions({ {localize('com_config_reset_base')} )} + {showHistory && ( + + )} {showScope && } ); diff --git a/src/components/configuration/ConfigPage.version-freeze.test.tsx b/src/components/configuration/ConfigPage.version-freeze.test.tsx new file mode 100644 index 00000000..600be682 --- /dev/null +++ b/src/components/configuration/ConfigPage.version-freeze.test.tsx @@ -0,0 +1,647 @@ +import { createToast } from '@clickhouse/click-ui'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; + +/** + * ConfigPage freezes `frozenBaseVersion` from the ['baseConfig'] query and + * sends it as `expectedVersion` on every mutating action, so a stale admin's + * request 409s instead of silently overwriting a newer save. That freeze is + * only re-synced while there are no pending field edits (touchedPaths.size + * === 0) — but Reset, Import, and Restore are reachable in exactly that + * state (they're disabled while dirty), so without an additional guard a + * background refetch (30s staleTime elapsing, a window-focus refetch, an + * unrelated Langfuse save invalidating the same document) landing WHILE one + * of their dialogs is open would silently re-freeze a newer version, and the + * admin's eventual confirm would succeed against that version instead of the + * one they actually reviewed when they opened the dialog. + */ + +const mockNavigate = vi.fn(); +const mockUseNavigate = vi.fn(() => mockNavigate); +vi.mock('@tanstack/react-router', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getRouteApi: (id: string) => { + if (id === '/_app/configuration/') { + return { useLoaderData: () => ({ tree: [] }) }; + } + if (id === '/_app') { + return { useRouteContext: () => ({ user: { id: 'admin-1', tenantId: 'tenant-1' } }) }; + } + return actual.getRouteApi(id as never); + }, + useBlocker: vi.fn(), + useNavigate: mockUseNavigate, + }; +}); + +vi.mock('@/hooks', async () => ({ + useConfigSession: ( + await vi.importActual('@/hooks/useConfigSession') + ).useConfigSession, + useLocalize: () => (key: string) => key, + useHighlightRef: () => () => {}, + useActiveSection: () => () => {}, + useCapabilities: () => ({ + capabilities: [], + hasCapability: () => true, + isLoading: false, + isError: false, + }), +})); + +vi.mock('@/components/configuration/ScopeSelector', () => ({ + ScopeSelector: () => null, + ScopeTriggerButton: () => null, +})); + +const mockResetBaseConfigFn = vi.fn().mockResolvedValue({ success: true }); +const mockGetBaseConfigFn = vi.fn(); +const mockConfigTabContent = vi.fn<(props: import('@/types').ConfigTabContentProps) => void>(); +const TENANT_BASE_QUERY_KEY = ['baseConfig', 'tenant-1'] as const; +const OTHER_TENANT_BASE_QUERY_KEY = ['baseConfig', 'tenant-2'] as const; + +// Observe the real page-to-renderer callbacks without replacing draft logic. +vi.mock('./ConfigTabContent', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + ConfigTabContent: (props: import('@/types').ConfigTabContentProps) => { + mockConfigTabContent(props); + return ; + }, + }; +}); + +vi.mock('@/server', () => ({ + baseConfigOptions: { + queryKey: ['baseConfig'], + queryFn: () => mockGetBaseConfigFn(), + }, + getBaseConfigFn: () => mockGetBaseConfigFn(), + configRevisionsOptions: () => ({ + queryKey: ['configRevisions'], + queryFn: () => Promise.resolve({ revisions: [] }), + }), + availableScopesOptions: (tenantId: string) => ({ + queryKey: ['availableScopes', tenantId], + queryFn: () => Promise.resolve([]), + }), + getResolvedConfigFn: vi.fn(), + getBatchFieldProfilesFn: vi.fn(), + getLangfuseConnectionFn: vi.fn(), + LANGFUSE_CONNECTION_QUERY_KEY: ['adminLangfuseConnection'], + resetBaseConfigFn: (...args: unknown[]) => mockResetBaseConfigFn(...args), + setBaseConfigActiveFn: vi.fn(), + importBaseConfigFn: vi.fn(), + restoreConfigRevisionFn: vi.fn(), + saveBaseConfigFn: vi.fn(), + removeFieldProfileValueFn: vi.fn(), + tombstoneFieldProfileValueFn: vi.fn(), + bulkSaveProfileValuesFn: vi.fn(), + createGroupFn: vi.fn(), + createRoleFn: vi.fn(), + parseImportedYaml: vi.fn(), +})); + +interface MockButtonProps { + label?: string; + onClick?: () => void; + disabled?: boolean; +} + +vi.mock('@clickhouse/click-ui', () => ({ + createToast: vi.fn(), + Icon: () => null, + Button: ({ label, onClick, disabled }: MockButtonProps) => ( + + ), + Badge: ({ text }: { text: string }) => {text}, + Alert: ({ children }: { children?: React.ReactNode }) =>
{children}
, + Dialog: Object.assign( + ({ open, children }: { open: boolean; children: React.ReactNode }) => + open ?
{children}
: null, + { + Content: ({ title, children }: { title: string; children: React.ReactNode }) => ( + <> +

{title}

+ {children} + + ), + }, + ), + Tabs: Object.assign(({ children }: { children?: React.ReactNode }) =>
{children}
, { + TriggersList: ({ children }: { children?: React.ReactNode }) =>
{children}
, + Trigger: ({ children }: { children?: React.ReactNode }) => , + }), + MultiAccordion: Object.assign( + ({ children }: { children?: React.ReactNode }) =>
{children}
, + { Item: ({ children }: { children?: React.ReactNode }) =>
{children}
}, + ), +})); + +// ConfigPage's Header* portal target: rendered by src/components/Header.tsx +// in the real app, which ConfigPage never mounts itself — without this div, +// the Reset/Import/History buttons never appear at all. +function ensureHeaderPortalTarget() { + if (!document.getElementById('header-actions-portal')) { + const portal = document.createElement('div'); + portal.id = 'header-actions-portal'; + document.body.appendChild(portal); + } +} + +function ensureLocalStorage() { + if (typeof window.localStorage !== 'undefined') return; + const store = new Map(); + vi.stubGlobal('localStorage', { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => store.set(key, value), + removeItem: (key: string) => store.delete(key), + clear: () => store.clear(), + }); +} + +async function renderConfigPage(dbConfigVersion: number) { + ensureHeaderPortalTarget(); + ensureLocalStorage(); + mockGetBaseConfigFn.mockResolvedValue({ + config: {}, + dbOverrides: { 'interface.modelSelect': true }, + dbConfigVersion, + dbIsActive: true, + effectiveTenantId: 'tenant-1', + configuredFromBase: [], + schemaDefaults: {}, + yamlMcpKeys: undefined, + yamlMcpServers: undefined, + }); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const { ConfigPage } = await import('./ConfigPage'); + render( + + + , + ); + return { queryClient }; +} + +describe('ConfigPage — frozen version protection for destructive actions', () => { + beforeEach(() => { + vi.clearAllMocks(); + ensureLocalStorage(); + window.localStorage.clear(); + mockResetBaseConfigFn.mockResolvedValue({ success: true }); + }); + + it('resetting a parent drops child edits from both the draft and touched paths', async () => { + await renderConfigPage(5); + await screen.findByRole('button', { name: 'com_config_reset_base' }); + const parent = 'speech.speechTab.textToSpeech'; + await act(async () => { + mockConfigTabContent.mock.lastCall![0].onFieldChange(`${parent}.voice`, 'nova'); + }); + expect(mockConfigTabContent.mock.lastCall![0].editedValues).toEqual({ + [`${parent}.voice`]: 'nova', + }); + await act(async () => { + mockConfigTabContent.mock.lastCall![0].onResetField!(parent); + }); + const props = mockConfigTabContent.mock.lastCall![0]; + expect(props.editedValues).toEqual({ [parent]: undefined }); + expect(props.touchedPaths).toEqual(new Set([parent])); + }); + + it('blocks a descendant edit after a parent reset and explains how to proceed', async () => { + await renderConfigPage(5); + await screen.findByRole('button', { name: 'com_config_reset_base' }); + const parent = 'speech.speechTab.textToSpeech'; + await act(async () => { + mockConfigTabContent.mock.lastCall![0].onResetField!(parent); + }); + await act(async () => { + mockConfigTabContent.mock.lastCall![0].onFieldChange(`${parent}.voice`, 'nova'); + }); + const props = mockConfigTabContent.mock.lastCall![0]; + expect(props.editedValues).toEqual({ [parent]: undefined }); + expect(props.touchedPaths).toEqual(new Set([parent])); + expect(createToast).toHaveBeenCalledWith( + expect.objectContaining({ title: 'com_config_reset_before_edit' }), + ); + }); + + it('sends the version captured when Reset opened, not a version that arrived via a background refetch while the dialog was open', async () => { + const { queryClient } = await renderConfigPage(5); + + const resetButton = await screen.findByRole('button', { name: 'com_config_reset_base' }); + fireEvent.click(resetButton); + await screen.findByRole('dialog'); + + // Simulate Admin B's save landing and Admin A's query refetching while + // the Reset dialog is still open (window focus, staleTime elapsing, or + // an unrelated Langfuse save invalidating the same shared document). + await act(async () => { + queryClient.setQueryData( + TENANT_BASE_QUERY_KEY, + (old: { dbConfigVersion: number } | undefined) => + old ? { ...old, dbConfigVersion: 6 } : old, + ); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + const confirmButton = screen.getByRole('button', { name: 'com_config_reset_base_action' }); + fireEvent.click(confirmButton); + + await waitFor(() => expect(mockResetBaseConfigFn).toHaveBeenCalledTimes(1)); + expect(mockResetBaseConfigFn).toHaveBeenCalledWith({ + data: { expectedVersion: 5, expectedTenantId: 'tenant-1' }, + }); + }); + + it('picks up a version change that happens BEFORE the dialog opens (freeze only applies once open)', async () => { + const { queryClient } = await renderConfigPage(5); + await screen.findByRole('button', { name: 'com_config_reset_base' }); + + // Unlike the previous test, this update happens while nothing is open — + // the resync effect should still be live and adopt it. + await act(async () => { + queryClient.setQueryData( + TENANT_BASE_QUERY_KEY, + (old: { dbConfigVersion: number } | undefined) => + old ? { ...old, dbConfigVersion: 7 } : old, + ); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + fireEvent.click(screen.getByRole('button', { name: 'com_config_reset_base' })); + await screen.findByRole('dialog'); + fireEvent.click(screen.getByRole('button', { name: 'com_config_reset_base_action' })); + + await waitFor(() => expect(mockResetBaseConfigFn).toHaveBeenCalledTimes(1)); + expect(mockResetBaseConfigFn).toHaveBeenCalledWith({ + data: { expectedVersion: 7, expectedTenantId: 'tenant-1' }, + }); + }); + + it('closes a destructive dialog instead of carrying an equal-version action into another tenant', async () => { + window.localStorage.setItem('config:lastScope', 'role:tenant-a-scope'); + const { queryClient } = await renderConfigPage(5); + fireEvent.click(await screen.findByRole('button', { name: 'com_config_reset_base' })); + await screen.findByRole('dialog'); + + const tenantTwo = { + config: {}, + dbOverrides: { 'interface.modelSelect': false }, + dbConfigVersion: 5, + dbIsActive: true, + effectiveTenantId: 'tenant-2', + configuredFromBase: [], + schemaDefaults: {}, + yamlMcpKeys: undefined, + yamlMcpServers: undefined, + }; + mockGetBaseConfigFn.mockResolvedValue(tenantTwo); + await act(async () => { + queryClient.setQueryData(TENANT_BASE_QUERY_KEY, tenantTwo); + await Promise.resolve(); + }); + + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + expect(mockResetBaseConfigFn).not.toHaveBeenCalled(); + expect(vi.mocked(createToast)).toHaveBeenCalledWith( + expect.objectContaining({ type: 'danger' }), + ); + await waitFor(() => + expect(queryClient.getQueryData(OTHER_TENANT_BASE_QUERY_KEY)).toEqual(tenantTwo), + ); + expect(queryClient.getQueryData(TENANT_BASE_QUERY_KEY)).toBeUndefined(); + expect(window.localStorage.getItem('config:lastScope')).toBeNull(); + const tenantResetNavigation = mockNavigate.mock.calls.find(([options]) => { + if (typeof options?.search !== 'function') return false; + return options.search({ scope: 'role:tenant-a-scope' }).scope === undefined; + }); + expect(tenantResetNavigation).toBeDefined(); + }); + + it('re-homes tenant discovery responses without poisoning cache keys across A-to-B-to-A', async () => { + const { queryClient } = await renderConfigPage(5); + await screen.findByRole('button', { name: 'com_config_reset_base' }); + + const tenantTwo = { + config: { marker: 'tenant-two' }, + dbOverrides: {}, + dbConfigVersion: 1, + dbIsActive: true, + effectiveTenantId: 'tenant-2', + configuredFromBase: [], + schemaDefaults: {}, + yamlMcpKeys: undefined, + yamlMcpServers: undefined, + }; + mockGetBaseConfigFn.mockResolvedValueOnce(tenantTwo); + await act(async () => { + await queryClient.refetchQueries({ queryKey: TENANT_BASE_QUERY_KEY, exact: true }); + }); + await waitFor(() => + expect(queryClient.getQueryData(OTHER_TENANT_BASE_QUERY_KEY)).toEqual(tenantTwo), + ); + expect(queryClient.getQueryData(TENANT_BASE_QUERY_KEY)).toBeUndefined(); + + const tenantOne = { + ...tenantTwo, + config: { marker: 'tenant-one' }, + dbConfigVersion: 6, + effectiveTenantId: 'tenant-1', + }; + mockGetBaseConfigFn.mockResolvedValueOnce(tenantOne); + await act(async () => { + await queryClient.refetchQueries({ queryKey: OTHER_TENANT_BASE_QUERY_KEY, exact: true }); + }); + + await waitFor(() => expect(queryClient.getQueryData(TENANT_BASE_QUERY_KEY)).toEqual(tenantOne)); + expect(queryClient.getQueryData(OTHER_TENANT_BASE_QUERY_KEY)).toBeUndefined(); + expect(queryClient.getQueryCache().findAll({ queryKey: ['baseConfig'] })).toHaveLength(1); + }); + + it('does not let an older tracked result regress the base-config cache', async () => { + const { queryClient } = await renderConfigPage(10); + await screen.findByRole('button', { name: 'com_config_reset_base' }); + const current = queryClient.getQueryData<{ + config: object; + dbOverrides: object; + dbConfigVersion: number; + }>(TENANT_BASE_QUERY_KEY); + expect(current?.dbConfigVersion).toBe(10); + + act(() => { + queryClient.setQueryData(TENANT_BASE_QUERY_KEY, { + ...current, + config: { marker: 'stale' }, + dbConfigVersion: 9, + }); + }); + + expect(queryClient.getQueryData(TENANT_BASE_QUERY_KEY)).toBe(current); + }); + + it('discard closes the dialog that caused the conflict and adopts the fresh version, so a retry does not 409 again', async () => { + // Reset (like Import and Restore) is only reachable while touchedPaths is + // empty, so handleDiscardAfterConflict's own dirty-edit clearing never + // protects it. Before this fix, discard only closed the version-conflict + // dialog itself — ResetBaseConfigDialog stayed open, and the frozen- + // version re-sync effect (gated on !hasDestructiveDialogOpen) never fired, + // so clicking confirm again resent the same stale frozenBaseVersion and + // 409ed forever. + await renderConfigPage(5); + + const resetButton = await screen.findByRole('button', { name: 'com_config_reset_base' }); + fireEvent.click(resetButton); + await screen.findByRole('dialog'); + + const conflictError = Object.assign(new Error('Config version conflict'), { + name: 'ConfigVersionConflictError', + }); + mockResetBaseConfigFn.mockRejectedValueOnce(conflictError); + mockGetBaseConfigFn.mockResolvedValueOnce({ + config: {}, + dbOverrides: { 'interface.modelSelect': true }, + dbConfigVersion: 6, + dbIsActive: true, + effectiveTenantId: 'tenant-1', + configuredFromBase: [], + schemaDefaults: {}, + yamlMcpKeys: undefined, + yamlMcpServers: undefined, + }); + + fireEvent.click(screen.getByRole('button', { name: 'com_config_reset_base_action' })); + await waitFor(() => expect(mockResetBaseConfigFn).toHaveBeenCalledTimes(1)); + await screen.findByRole('button', { name: 'com_config_version_conflict_discard' }); + // The version-conflict dialog and the Reset dialog that triggered it are both open. + expect(screen.getAllByRole('dialog')).toHaveLength(2); + + fireEvent.click(screen.getByRole('button', { name: 'com_config_version_conflict_discard' })); + + await waitFor(() => expect(screen.queryAllByRole('dialog')).toHaveLength(0)); + + // Retrying Reset must use the freshly adopted version, not 409 again on + // the same stale one. + mockResetBaseConfigFn.mockResolvedValueOnce({ success: true }); + fireEvent.click(screen.getByRole('button', { name: 'com_config_reset_base' })); + await screen.findByRole('dialog'); + fireEvent.click(screen.getByRole('button', { name: 'com_config_reset_base_action' })); + + await waitFor(() => expect(mockResetBaseConfigFn).toHaveBeenCalledTimes(2)); + expect(mockResetBaseConfigFn).toHaveBeenLastCalledWith({ + data: { expectedVersion: 6, expectedTenantId: 'tenant-1' }, + }); + }); + + it('discard surfaces an error and leaves both dialogs open when the fresh-base fetch fails', async () => { + // Before this fix, handleDiscardAfterConflict's try/finally had no catch: + // a rejection from the fresh-base fetch left the async click handler's + // promise rejecting silently, with no notification and both dialogs + // (and discardingConflict) stuck exactly where they were. + await renderConfigPage(5); + + const resetButton = await screen.findByRole('button', { name: 'com_config_reset_base' }); + fireEvent.click(resetButton); + await screen.findByRole('dialog'); + + const conflictError = Object.assign(new Error('Config version conflict'), { + name: 'ConfigVersionConflictError', + }); + mockResetBaseConfigFn.mockRejectedValueOnce(conflictError); + + fireEvent.click(screen.getByRole('button', { name: 'com_config_reset_base_action' })); + await waitFor(() => expect(mockResetBaseConfigFn).toHaveBeenCalledTimes(1)); + await screen.findByRole('button', { name: 'com_config_version_conflict_discard' }); + + mockGetBaseConfigFn.mockRejectedValueOnce(new Error('network blip')); + + fireEvent.click(screen.getByRole('button', { name: 'com_config_version_conflict_discard' })); + + await waitFor(() => + expect(vi.mocked(createToast)).toHaveBeenCalledWith( + expect.objectContaining({ type: 'danger' }), + ), + ); + expect(screen.getAllByRole('dialog')).toHaveLength(2); + expect( + screen.getByRole('button', { name: 'com_config_version_conflict_discard' }), + ).toBeEnabled(); + }); + + it('discard resolves with its own independent call even while a same-key fetchQuery is still in flight', async () => { + // React Query's fetchQuery joins an already-in-flight request for the + // same key regardless of staleTime, so a discard implemented on top of + // fetchQuery (instead of calling getBaseConfigFn directly) could adopt + // whatever a still-pending, pre-conflict background refetch eventually + // resolves with -- stale content and version reinstalled right after + // the admin was told the conflict was resolved. + const { queryClient } = await renderConfigPage(5); + + const resetButton = await screen.findByRole('button', { name: 'com_config_reset_base' }); + fireEvent.click(resetButton); + await screen.findByRole('dialog'); + + const conflictError = Object.assign(new Error('Config version conflict'), { + name: 'ConfigVersionConflictError', + }); + mockResetBaseConfigFn.mockRejectedValueOnce(conflictError); + + fireEvent.click(screen.getByRole('button', { name: 'com_config_reset_base_action' })); + await waitFor(() => expect(mockResetBaseConfigFn).toHaveBeenCalledTimes(1)); + await screen.findByRole('button', { name: 'com_config_version_conflict_discard' }); + + // A same-key fetchQuery is still pending when discard is triggered. + let resolvePendingFetch: + | ((value: { config: object; dbConfigVersion: number }) => void) + | undefined; + mockGetBaseConfigFn.mockReturnValueOnce( + new Promise((resolve) => { + resolvePendingFetch = resolve; + }), + ); + const staleInFlight = queryClient.fetchQuery({ + queryKey: TENANT_BASE_QUERY_KEY, + queryFn: () => mockGetBaseConfigFn(), + }); + mockGetBaseConfigFn.mockResolvedValueOnce({ + config: {}, + dbOverrides: { 'interface.modelSelect': true }, + dbConfigVersion: 9, + dbIsActive: true, + effectiveTenantId: 'tenant-1', + configuredFromBase: [], + schemaDefaults: {}, + yamlMcpKeys: undefined, + yamlMcpServers: undefined, + }); + + fireEvent.click(screen.getByRole('button', { name: 'com_config_version_conflict_discard' })); + + await waitFor(() => expect(screen.queryAllByRole('dialog')).toHaveLength(0)); + // Discard adopted version 9 from its own direct call, not the version-5 + // response the still-pending fetchQuery above will eventually resolve with. + expect( + (queryClient.getQueryData(TENANT_BASE_QUERY_KEY) as { dbConfigVersion: number }) + .dbConfigVersion, + ).toBe(9); + + // Settling the stale in-flight fetch afterward must not silently revert + // the cache: calling getBaseConfigFn directly stops discard from + // *adopting* a stale in-flight response, but does nothing on its own to + // stop that older request from resolving later and overwriting what was + // just installed -- only cancelling it first does that. + await act(async () => { + resolvePendingFetch?.({ + config: {}, + dbConfigVersion: 5, + effectiveTenantId: 'tenant-1', + } as never); + await staleInFlight.catch(() => undefined); + }); + expect( + (queryClient.getQueryData(TENANT_BASE_QUERY_KEY) as { dbConfigVersion: number }) + .dbConfigVersion, + ).toBe(9); + }); + + it('does not let a tracked fetch that starts during the direct read get wrongly cancelled', async () => { + // Cancelling AFTER the direct read (instead of before) would discard a + // tracked fetch that starts while that read is still in flight -- even + // when that fetch resolves with a genuinely newer version than the + // direct read's own result. Cancelling first means such a fetch was + // never tracked at cancel-time, so it survives to land afterward. + const { queryClient } = await renderConfigPage(5); + + const resetButton = await screen.findByRole('button', { name: 'com_config_reset_base' }); + fireEvent.click(resetButton); + await screen.findByRole('dialog'); + + const conflictError = Object.assign(new Error('Config version conflict'), { + name: 'ConfigVersionConflictError', + }); + mockResetBaseConfigFn.mockRejectedValueOnce(conflictError); + fireEvent.click(screen.getByRole('button', { name: 'com_config_reset_base_action' })); + await waitFor(() => expect(mockResetBaseConfigFn).toHaveBeenCalledTimes(1)); + await screen.findByRole('button', { name: 'com_config_version_conflict_discard' }); + + // Discard's own direct read is held pending. + let resolveDiscardRead: + | ((value: { config: object; dbConfigVersion: number }) => void) + | undefined; + mockGetBaseConfigFn.mockReturnValueOnce( + new Promise((resolve) => { + resolveDiscardRead = resolve; + }), + ); + const callsBeforeDiscard = mockGetBaseConfigFn.mock.calls.length; + fireEvent.click(screen.getByRole('button', { name: 'com_config_version_conflict_discard' })); + // Waits for discard's own getBaseConfigFn() call to actually fire, which + // -- with cancelQueries positioned before it -- only happens once that + // cancellation has already completed. + await waitFor(() => expect(mockGetBaseConfigFn.mock.calls.length).toBe(callsBeforeDiscard + 1)); + + // A tracked fetch starts WHILE discard's own read is still pending -- an + // unrelated background refetch firing at the same moment. It will + // resolve with a genuinely newer version than discard's own read. + let resolveTrackedFetch: + | ((value: { config: object; dbConfigVersion: number }) => void) + | undefined; + mockGetBaseConfigFn.mockReturnValueOnce( + new Promise((resolve) => { + resolveTrackedFetch = resolve; + }), + ); + const trackedFetch = queryClient.fetchQuery({ + queryKey: TENANT_BASE_QUERY_KEY, + queryFn: () => mockGetBaseConfigFn(), + }); + await waitFor(() => expect(mockGetBaseConfigFn.mock.calls.length).toBe(callsBeforeDiscard + 2)); + + // Discard's own read resolves with an OLDER version than the tracked + // fetch above will. + await act(async () => { + resolveDiscardRead?.({ + config: {}, + dbOverrides: {}, + dbConfigVersion: 9, + dbIsActive: true, + effectiveTenantId: 'tenant-1', + configuredFromBase: [], + schemaDefaults: {}, + yamlMcpKeys: undefined, + yamlMcpServers: undefined, + } as never); + }); + await waitFor(() => expect(screen.queryAllByRole('dialog')).toHaveLength(0)); + + // The tracked fetch, never cancelled, is free to land afterward and win + // on version. + await act(async () => { + resolveTrackedFetch?.({ + config: {}, + dbOverrides: {}, + dbConfigVersion: 12, + dbIsActive: true, + effectiveTenantId: 'tenant-1', + configuredFromBase: [], + schemaDefaults: {}, + yamlMcpKeys: undefined, + yamlMcpServers: undefined, + } as never); + await trackedFetch; + }); + + expect( + (queryClient.getQueryData(TENANT_BASE_QUERY_KEY) as { dbConfigVersion: number }) + .dbConfigVersion, + ).toBe(12); + }); +}); diff --git a/src/components/configuration/ConfigTabContent.tsx b/src/components/configuration/ConfigTabContent.tsx index fa0fcd7e..2e4a8dc5 100644 --- a/src/components/configuration/ConfigTabContent.tsx +++ b/src/components/configuration/ConfigTabContent.tsx @@ -56,6 +56,7 @@ export function ConfigTabContent({ schemaDefaults, showConfiguredOnly, isEditingScope, + effectiveTenantId, baseRecordKeys, onValidationError, editSessionId, @@ -187,6 +188,7 @@ export function ConfigTabContent({ schemaDefaults, showConfiguredOnly, isEditingScope, + effectiveTenantId, yamlBaseKeys: baseRecordKeys?.[dataKey], onValidationError, editSessionId, diff --git a/src/components/configuration/FieldProfilePopover.tsx b/src/components/configuration/FieldProfilePopover.tsx index f495d964..13f6401f 100644 --- a/src/components/configuration/FieldProfilePopover.tsx +++ b/src/components/configuration/FieldProfilePopover.tsx @@ -14,6 +14,7 @@ import { getControlType } from './utils'; export function FieldProfilePopover({ fieldPath, fieldLabel, + expectedTenantId, fieldSchema, profileValues, permissions, @@ -26,7 +27,7 @@ export function FieldProfilePopover({ const [selectedAddScope, setSelectedAddScope] = useState(null); const [deleteScope, setDeleteScope] = useState(null); - const { data: allScopes = [] } = useQuery(availableScopesOptions); + const { data: allScopes = [] } = useQuery(availableScopesOptions(expectedTenantId)); const availableScopes = useMemo(() => { const existingKeys = new Set( @@ -47,6 +48,7 @@ export function FieldProfilePopover({ const { saveMutation, removeMutation, saving } = useProfileMutations({ fieldPath, + expectedTenantId, onProfileChange, }); @@ -71,7 +73,7 @@ export function FieldProfilePopover({ const handleModalSave = useCallback(() => { if (modalIsBase && onBaseValueChange) { - onBaseValueChange(serializeKVPairs(modalValue)); + onBaseValueChange(serializeKVPairs(modalValue, fieldPath)); setModalOpen(false); setModalIsBase(false); return; @@ -81,7 +83,7 @@ export function FieldProfilePopover({ { principalType: modalScope.principalType, principalId: modalScope.principalId, - value: serializeKVPairs(modalValue), + value: serializeKVPairs(modalValue, fieldPath), }, { onSuccess: () => { @@ -94,7 +96,7 @@ export function FieldProfilePopover({ }, }, ); - }, [modalIsBase, modalScope, modalValue, modalMode, saveMutation, onBaseValueChange]); + }, [modalIsBase, modalScope, modalValue, modalMode, saveMutation, onBaseValueChange, fieldPath]); const handleModalCancel = useCallback(() => { setModalOpen(false); diff --git a/src/components/configuration/FieldRenderer.tsx b/src/components/configuration/FieldRenderer.tsx index bc1e6d0b..cfdb11f8 100644 --- a/src/components/configuration/FieldRenderer.tsx +++ b/src/components/configuration/FieldRenderer.tsx @@ -23,6 +23,7 @@ import { ListRecordField } from './fields/ListRecordField'; import { renderCollapsible } from './renderCollapsible'; import { TextareaField } from './fields/TextareaField'; import { KeyValueField } from './fields/KeyValueField'; +import { cn, getSecretPreviewValue } from '@/utils'; import { NumberField } from './fields/NumberField'; import { SecretField } from './fields/SecretField'; import { ToggleField } from './fields/ToggleField'; @@ -32,7 +33,17 @@ import { ListField } from './fields/ListField'; import { CodeField } from './fields/CodeField'; import { ConfigRow } from './ConfigRow'; import { useLocalize } from '@/hooks'; -import { cn, getSecretPreviewValue } from '@/utils'; + +/** + * Array-of-object schema paths whose items carry a stable identity field used + * for encrypted-credential preservation on the backend (`endpoints.custom` is + * wired directly in its own dedicated renderer). Lets `ArrayObjectField` + * attach a pre-edit identity hint so renaming the field doesn't strand the + * entry's apiKey/headers — see `ArrayObjectField`'s `identityKey` prop. + */ +const ARRAY_SECRET_IDENTITY_KEYS: Record = { + 'endpoints.azureOpenAI.groups': 'group', +}; function formatDefault(value: t.ConfigValue): string | null { if (value === undefined || value === null) return null; @@ -68,6 +79,7 @@ function ArrayObjectNestedGroup({ const items = Array.isArray(currentValue) ? currentValue : []; const addTriggerRef = useRef<(() => void) | null>(null); const handleAdd = disabled ? undefined : () => addTriggerRef.current?.(); + const identityKey = ARRAY_SECRET_IDENTITY_KEYS[field.path]; const handleEntryChange = useCallback( (index: number, value: t.ConfigValue) => onChange(`${path}.${index}`, value), @@ -76,6 +88,12 @@ function ArrayObjectNestedGroup({ const arrayField = ( ); if (isSoleField) return arrayField; @@ -481,6 +500,7 @@ export function SingleFieldRenderer({ disabled={disabled} valueTypes={field.recordValueKVTypes} aria-label={fieldLabel} + fieldPath={path} /> ); @@ -1225,6 +1245,7 @@ export function renderInlineField( disabled={disabled} valueTypes={field.recordValueKVTypes} aria-label={fieldLabel} + fieldPath={`${parentPath}.${field.key}`} /> ); diff --git a/src/components/configuration/ImportYamlDialog.tsx b/src/components/configuration/ImportYamlDialog.tsx index 0cdd5ae9..2ac56dc9 100644 --- a/src/components/configuration/ImportYamlDialog.tsx +++ b/src/components/configuration/ImportYamlDialog.tsx @@ -10,6 +10,7 @@ import { cn } from '@/utils'; export function ImportYamlDialog({ open, + expectedTenantId, onClose, onImport, onImportAsProfile, @@ -34,7 +35,7 @@ export function ImportYamlDialog({ const [newScopeName, setNewScopeName] = useState(''); const { data: allScopes = [] } = useQuery({ - ...availableScopesOptions, + ...availableScopesOptions(expectedTenantId), enabled: open && step === 'target', }); @@ -146,7 +147,7 @@ export function ImportYamlDialog({ const name = newScopeName.trim(); if (newScopeType === PrincipalType.ROLE) { - const { role } = await createRoleFn({ data: { name } }); + const { role } = await createRoleFn({ data: { name, expectedTenantId } }); scope = { principalType: PrincipalType.ROLE, principalId: role.id, @@ -155,7 +156,9 @@ export function ImportYamlDialog({ isActive: true, }; } else { - const { group } = await createGroupFn({ data: { name, description: '' } }); + const { group } = await createGroupFn({ + data: { name, description: '', expectedTenantId }, + }); scope = { principalType: PrincipalType.GROUP, principalId: group.id, diff --git a/src/components/configuration/PreviewProfileActions.tsx b/src/components/configuration/PreviewProfileActions.tsx index 7c5f0d60..1d94b8ad 100644 --- a/src/components/configuration/PreviewProfileActions.tsx +++ b/src/components/configuration/PreviewProfileActions.tsx @@ -9,6 +9,7 @@ import { getControlType } from './utils'; export function PreviewProfileActions({ fieldPath, fieldLabel, + expectedTenantId, fieldSchema, scope, currentValue, @@ -27,6 +28,7 @@ export function PreviewProfileActions({ saving: busy, } = useProfileMutations({ fieldPath, + expectedTenantId, onProfileChange, }); diff --git a/src/components/configuration/ProfileIndicator.test.tsx b/src/components/configuration/ProfileIndicator.test.tsx new file mode 100644 index 00000000..3cf4f27f --- /dev/null +++ b/src/components/configuration/ProfileIndicator.test.tsx @@ -0,0 +1,92 @@ +import { describe, expect, it, vi } from 'vitest'; +import { PrincipalType } from 'librechat-data-provider'; +import { fireEvent, render, screen } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import type { ReactNode } from 'react'; +import type * as t from '@/types'; +import { ProfileIndicator } from './ProfileIndicator'; + +const fetchProfileValuesMock = vi.hoisted(() => vi.fn()); + +vi.mock('@/server', () => ({ + fieldProfileValuesOptions: (fieldPath: string, expectedTenantId: string) => ({ + queryKey: ['fieldProfileValues', expectedTenantId, fieldPath], + queryFn: fetchProfileValuesMock, + }), + tenantQueryKeys: { + fieldProfileValues: (tenantId: string, fieldPath: string) => + ['fieldProfileValues', tenantId, fieldPath] as const, + }, +})); + +vi.mock('@/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +vi.mock('@clickhouse/click-ui', () => { + const Dialog = Object.assign(({ children }: { children: ReactNode }) => <>{children}, { + Content: ({ children }: { children: ReactNode }) =>
{children}
, + }); + return { Dialog, Icon: () => null }; +}); + +vi.mock('./FieldProfilePopover', () => ({ + FieldProfilePopover: ({ + profileValues, + onProfileChange, + }: { + profileValues: t.FieldProfileValue[]; + onProfileChange: () => void; + }) => ( +
+ {String(profileValues[0]?.value ?? '')} + +
+ ), +})); + +function profileValue(value: string): t.FieldProfileValue { + return { + scope: { + principalType: PrincipalType.ROLE, + principalId: 'role-1', + name: 'Role 1', + priority: 100, + isActive: true, + }, + value, + }; +} + +describe('ProfileIndicator', () => { + it('refreshes tenant-scoped profile values while the parent dialog remains open', async () => { + fetchProfileValuesMock + .mockResolvedValueOnce([profileValue('old')]) + .mockResolvedValueOnce([profileValue('new')]); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + render( + + + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'com_scope_field_profiles: MCP servers' })); + await screen.findByText('old'); + + fireEvent.click(screen.getByRole('button', { name: 'mutate profile' })); + await screen.findByText('new'); + + expect(fetchProfileValuesMock).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/components/configuration/ProfileIndicator.tsx b/src/components/configuration/ProfileIndicator.tsx index c7a499a9..71ae8640 100644 --- a/src/components/configuration/ProfileIndicator.tsx +++ b/src/components/configuration/ProfileIndicator.tsx @@ -3,14 +3,15 @@ import { Icon, Dialog } from '@clickhouse/click-ui'; import { PrincipalType } from 'librechat-data-provider'; import { useQuery, useQueryClient } from '@tanstack/react-query'; import type * as t from '@/types'; +import { fieldProfileValuesOptions, tenantQueryKeys } from '@/server'; import { FieldProfilePopover } from './FieldProfilePopover'; -import { fieldProfileValuesOptions } from '@/server'; import { getScopeTypeConfig } from '@/constants'; import { useLocalize } from '@/hooks'; export function ProfileIndicator({ fieldPath, fieldLabel, + expectedTenantId, fieldSchema, profileTypes, permissions, @@ -25,7 +26,7 @@ export function ProfileIndicator({ const hasProfiles = profileTypes && profileTypes.length > 0; const { data: profileValues = [] } = useQuery({ - ...fieldProfileValuesOptions(fieldPath), + ...fieldProfileValuesOptions(fieldPath, expectedTenantId), enabled: dialogOpen, }) as { data: t.FieldProfileValue[] }; @@ -34,9 +35,11 @@ export function ProfileIndicator({ }, []); const handleProfileChange = useCallback(() => { - queryClient.invalidateQueries({ queryKey: ['fieldProfileValues', fieldPath] }); + queryClient.invalidateQueries({ + queryKey: tenantQueryKeys.fieldProfileValues(expectedTenantId, fieldPath), + }); onProfileChange?.(); - }, [queryClient, fieldPath, onProfileChange]); + }, [queryClient, expectedTenantId, fieldPath, onProfileChange]); if (!hasProfiles) return null; @@ -94,6 +97,7 @@ export function ProfileIndicator({ = { + save: 'com_config_revision_cause_save', + import: 'com_config_revision_cause_import', + reset: 'com_config_revision_cause_reset', + restore: 'com_config_revision_cause_restore', +}; + +function formatTimestamp(iso: string): string { + try { + return new Intl.DateTimeFormat(undefined, { + year: 'numeric', + month: 'short', + day: 'numeric', + hour: '2-digit', + minute: '2-digit', + }).format(new Date(iso)); + } catch { + return iso; + } +} + +export function RevisionHistoryDialog({ + open, + loading, + restoring, + error, + revisions, + onRestore, + onCancel, +}: t.RevisionHistoryDialogProps) { + const localize = useLocalize(); + const [pendingId, setPendingId] = useState(null); + const pending = revisions.find((revision) => revision.id === pendingId); + + useEffect(() => { + if (!open) setPendingId(null); + }, [open]); + + return ( + { + if (!isOpen) { + setPendingId(null); + onCancel(); + } + }} + > + { + setPendingId(null); + onCancel(); + }} + className="modal-frost" + > +
+

+ {localize('com_config_revision_desc')} +

+ + {loading && ( +

{localize('com_ui_loading')}

+ )} + + {!loading && revisions.length === 0 && ( +

+ {localize('com_config_revision_empty')} +

+ )} + + {!loading && revisions.length > 0 && ( +
+ {revisions.map((revision) => ( +
+
+
+ + {formatTimestamp(revision.createdAt)} + + +
+

+ {revision.actorEmail ?? revision.actorId} +

+
+
+ ))} +
+ )} + + {pending && ( +
+

+ {localize('com_config_revision_confirm', { + time: formatTimestamp(pending.createdAt), + })} +

+
+
+
+ )} + + {error && ( +
+ {error} +
+ )} + + {!pending && ( +
+
+ )} +
+ + + ); +} diff --git a/src/components/configuration/ScopeSelector.tsx b/src/components/configuration/ScopeSelector.tsx index 9af2d913..57eb14ee 100644 --- a/src/components/configuration/ScopeSelector.tsx +++ b/src/components/configuration/ScopeSelector.tsx @@ -9,8 +9,8 @@ import type { AdminGroup } from '@librechat/data-schemas'; import type * as t from '@/types'; import { availableScopesOptions, - allRolesQueryOptions, - allGroupsQueryOptions, + allRolesForTenantQueryOptions, + allGroupsForTenantQueryOptions, createScopeFn, deleteScopeFn, } from '@/server'; @@ -22,6 +22,7 @@ import { cn } from '@/utils'; export function ScopeSelector({ open, + expectedTenantId, onOpenChange, currentSelection, onSelect, @@ -38,17 +39,17 @@ export function ScopeSelector({ const listRef = useRef(null); const { data: scopes = [], isLoading: loading } = useQuery({ - ...availableScopesOptions, + ...availableScopesOptions(expectedTenantId), enabled: open, }); const { data: allRoles = [] } = useQuery({ - ...allRolesQueryOptions, + ...allRolesForTenantQueryOptions(expectedTenantId), enabled: open && showCreate, }); const { data: allGroups = [] } = useQuery({ - ...allGroupsQueryOptions, + ...allGroupsForTenantQueryOptions(expectedTenantId), enabled: open && showCreate, }); @@ -119,6 +120,7 @@ export function ScopeSelector({ name: role.name, priority: 10, principalId: role.id, + expectedTenantId, }, }); await queryClient.invalidateQueries({ queryKey: ['availableScopes'] }); @@ -128,7 +130,7 @@ export function ScopeSelector({ onError?.(err instanceof Error ? err.message : localize('com_scope_create_error')); } }, - [creating, queryClient, resetState, onError, localize], + [creating, expectedTenantId, queryClient, resetState, onError, localize], ); const handleCreateForGroup = useCallback( @@ -142,6 +144,7 @@ export function ScopeSelector({ name: group.name, priority: 20, principalId: group.id, + expectedTenantId, }, }); await queryClient.invalidateQueries({ queryKey: ['availableScopes'] }); @@ -151,7 +154,7 @@ export function ScopeSelector({ onError?.(err instanceof Error ? err.message : localize('com_scope_create_error')); } }, - [creating, queryClient, resetState, onError, localize], + [creating, expectedTenantId, queryClient, resetState, onError, localize], ); const handleDelete = useCallback(async () => { @@ -162,6 +165,7 @@ export function ScopeSelector({ data: { principalType: deleteTarget.principalType, principalId: deleteTarget.principalId, + expectedTenantId, }, }); await queryClient.invalidateQueries({ queryKey: ['availableScopes'] }); @@ -178,7 +182,16 @@ export function ScopeSelector({ setDeleting(false); onError?.(err instanceof Error ? err.message : localize('com_scope_delete_error')); } - }, [deleteTarget, deleting, queryClient, currentSelection, onSelect, onError, localize]); + }, [ + deleteTarget, + deleting, + expectedTenantId, + queryClient, + currentSelection, + onSelect, + onError, + localize, + ]); const roleScopes = useMemo( () => scopes.filter((s) => s.principalType === PrincipalType.ROLE), diff --git a/src/components/configuration/VersionConflictDialog.tsx b/src/components/configuration/VersionConflictDialog.tsx new file mode 100644 index 00000000..c66ec56d --- /dev/null +++ b/src/components/configuration/VersionConflictDialog.tsx @@ -0,0 +1,53 @@ +import { Button, Dialog } from '@clickhouse/click-ui'; +import type * as t from '@/types'; +import { useLocalize } from '@/hooks'; + +export function VersionConflictDialog({ + open, + rebasing, + discarding, + onRebase, + onDiscard, +}: t.VersionConflictDialogProps) { + const localize = useLocalize(); + // Both actions are disabled while either is in flight — not just their own + // busy flag — so a click on the other button can't start a second, + // conflicting resolution (e.g. rebase landing new data mid-discard, or + // vice versa) while the awaited cache refresh from the first is pending. + const busy = rebasing || discarding; + + return ( + // Deliberately not dismissible via backdrop/escape: the admin must pick + // rebase or discard, or their next save just 409s again against the same + // stale frozen version. + {}}> + +
+

+ {localize('com_config_version_conflict_body')} +

+ +
+
+
+ + + ); +} diff --git a/src/components/configuration/fields/ArrayObjectField.test.tsx b/src/components/configuration/fields/ArrayObjectField.test.tsx new file mode 100644 index 00000000..f127b4c3 --- /dev/null +++ b/src/components/configuration/fields/ArrayObjectField.test.tsx @@ -0,0 +1,397 @@ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import type * as t from '@/types'; +import { ArrayObjectField } from './ArrayObjectField'; +import { createField } from '@/test/fixtures'; + +vi.mock('@/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +vi.mock('@clickhouse/click-ui', () => ({ + Icon: () => null, + IconButton: ({ + onClick, + 'aria-label': ariaLabel, + }: { + onClick?: () => void; + 'aria-label'?: string; + }) => + ), +})); + +const entryFields: t.SchemaField[] = [ + createField({ key: 'name', type: 'string' }), + createField({ key: 'baseURL', type: 'string' }), +]; + +/** Renders one button per entry that renames it to `newValue` when clicked. */ +const renderRenameButton: t.CollectionRenderFields = (_fields, _parentValue, entryKey, onChange) => ( + +); + +/** Renders one button per entry that edits an unrelated field (baseURL). */ +const renderUnrelatedEditButton: t.CollectionRenderFields = ( + _fields, + _parentValue, + entryKey, + onChange, +) => ; + +/** ObjectEntryCard only calls renderFields once its card is expanded. */ +function expandEntry(label: string) { + fireEvent.click(screen.getByText(label).closest('[role="button"]') as HTMLElement); +} + +describe('ArrayObjectField — __previousIdentity hint for renamed entries', () => { + it('attaches the original identity when the identity field is renamed', () => { + const onEntryChange = vi.fn(); + render( + , + ); + expandEntry('OpenRouter'); + fireEvent.click(screen.getByText('rename-OpenRouter')); + expect(onEntryChange).toHaveBeenCalledWith(0, { + name: 'renamed', + baseURL: 'https://old', + __previousIdentity: 'OpenRouter', + }); + }); + + it('also attaches the hint on an unrelated field edit, unchanged from the current identity', () => { + const onEntryChange = vi.fn(); + render( + , + ); + expandEntry('OpenRouter'); + fireEvent.click(screen.getByText('edit-OpenRouter')); + expect(onEntryChange).toHaveBeenCalledWith(0, { + name: 'OpenRouter', + baseURL: 'https://new', + __previousIdentity: 'OpenRouter', + }); + }); + + it('does not attach a hint when identityKey is not provided', () => { + const onEntryChange = vi.fn(); + render( + , + ); + expandEntry('OpenRouter'); + fireEvent.click(screen.getByText('rename-OpenRouter')); + expect(onEntryChange).toHaveBeenCalledWith(0, { name: 'renamed', baseURL: 'https://old' }); + }); + + it('stamps an explicit null origin for a pre-existing identity-less entry named for the first time', () => { + // A stored row with no name at all (e.g. saved blank in an earlier + // session) is wire-identical, at the moment it's first edited, to a + // brand-new entry added this session — neither has an embedded hint nor + // a current identity. Both must resolve to the same explicit "no + // origin" signal, or naming this row later in the same edit would let + // it fall back to bare-identity matching and inherit another entry's + // credentials merely by ending up with that entry's freed name. + const onEntryChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByRole('button', { expanded: false })); + fireEvent.click(screen.getByText(/^rename-/)); + expect(onEntryChange).toHaveBeenCalledWith(0, { + name: 'renamed', + __previousIdentity: null, + }); + }); + + it('keeps the true original identity across a second rename in the same session', () => { + const onEntryChange = vi.fn(); + const { rerender } = render( + , + ); + expandEntry('OpenRouter'); + fireEvent.click(screen.getByText('rename-OpenRouter')); + expect(onEntryChange).toHaveBeenLastCalledWith(0, { + name: 'renamed', + baseURL: 'https://old', + __previousIdentity: 'OpenRouter', + }); + + // The parent applies the first rename and re-renders with the new name. + rerender( + , + ); + fireEvent.click(screen.getByText('rename-renamed')); + expect(onEntryChange).toHaveBeenLastCalledWith(0, { + name: 'renamed', + baseURL: 'https://old', + __previousIdentity: 'OpenRouter', + }); + }); + + it('keeps the correct original identity for an existing entry after a sibling is prepended via Add entry', () => { + const onEntryChange = vi.fn(); + const onChange = vi.fn(); + const initialValue = [ + { name: 'OpenRouter', baseURL: 'https://old' }, + { name: 'Anyscale', baseURL: 'https://anyscale' }, + ]; + const { rerender } = render( + , + ); + + expandEntry('OpenRouter'); + + // Real "Add entry" flow: prepends a blank entry, shifting OpenRouter from + // index 0 to 1 and Anyscale from 1 to 2 — exercises the same key-sync + // path a real add goes through, unlike directly swapping the value prop. + fireEvent.click(screen.getByText('com_ui_add_item')); + expect(onChange).toHaveBeenCalledWith([{ __previousIdentity: null }, ...initialValue]); + rerender( + , + ); + + fireEvent.click(screen.getByText('rename-OpenRouter')); + expect(onEntryChange).toHaveBeenCalledWith(1, { + name: 'renamed', + baseURL: 'https://old', + __previousIdentity: 'OpenRouter', + }); + }); + + it('captures a fresh origin for the next session instead of reusing a stale hint from a completed save', () => { + // Session 1: rename OpenRouter -> OpenRouter EU, save succeeds. Session 2 + // (editSessionId bumped, value refetched to reflect the save): rename + // OpenRouter EU -> OpenRouter US. The hint must be "OpenRouter EU" — the + // entry's real predecessor this session — not the stale "OpenRouter" + // from before the save, which the backend can no longer find at all. + // + // The real call sites key `` so + // React fully remounts on a session change — `rerender` alone (same + // instance) wouldn't exercise that, so this simulates the remount with + // unmount+render instead, same as the test below. + const onEntryChange = vi.fn(); + const { unmount } = render( + , + ); + expandEntry('OpenRouter'); + fireEvent.click(screen.getByText('rename-OpenRouter')); + expect(onEntryChange).toHaveBeenLastCalledWith(0, { + name: 'renamed', + baseURL: 'https://old', + __previousIdentity: 'OpenRouter', + }); + unmount(); + + // Save succeeds: ConfigPage refetches, bumps editSessionId (remounting + // the keyed component), and the fresh baseline reflects the post-save + // name — a genuine server read, not a surviving draft, so it carries no + // embedded hint of its own. + render( + , + ); + expandEntry('OpenRouter EU'); + fireEvent.click(screen.getByText('rename-OpenRouter EU')); + expect(onEntryChange).toHaveBeenLastCalledWith(0, { + name: 'renamed', + baseURL: 'https://old', + __previousIdentity: 'OpenRouter EU', + }); + }); + + it('recovers the true origin from an already-embedded hint after a remount within the same session', () => { + // Switching config tabs and back unmounts and remounts ArrayObjectField + // without bumping editSessionId — a genuinely different trigger from the + // save/reset/restore/discard boundary above. The origin map starts empty + // again, but the draft value itself (from editedValues, unaffected by + // the remount) already carries the real origin as __previousIdentity — + // recapturing from the entry's current (already-renamed) name instead + // would silently replace the correct hint with a wrong one. + const onEntryChange = vi.fn(); + const { unmount } = render( + , + ); + expandEntry('OpenRouter'); + fireEvent.click(screen.getByText('rename-OpenRouter')); + expect(onEntryChange).toHaveBeenLastCalledWith(0, { + name: 'renamed', + baseURL: 'https://old', + __previousIdentity: 'OpenRouter', + }); + unmount(); + + // Tab switch and back: fresh component instance, same session, and the + // parent's draft value already has the previous edit's hint embedded. + render( + , + ); + expandEntry('renamed'); + fireEvent.click(screen.getByText('edit-renamed')); + expect(onEntryChange).toHaveBeenLastCalledWith(0, { + name: 'renamed', + baseURL: 'https://new', + __previousIdentity: 'OpenRouter', + }); + }); +}); + +describe('ArrayObjectField — strips untouched secret-record containers on structural edits', () => { + const fieldsWithHeaders: t.SchemaField[] = [ + ...entryFields, + createField({ key: 'headers', type: 'record', recordValueType: 'primitive' }), + ]; + + it('strips a redacted headers placeholder from a surviving entry when adding a new one', () => { + const onChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByText('com_ui_add_item')); + expect(onChange).toHaveBeenCalledWith([{}, { name: 'OpenRouter', baseURL: 'https://old' }]); + }); + + it('strips a redacted headers placeholder from a surviving entry when removing another', () => { + const onChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getAllByLabelText(/com_ui_delete/)[1]); + expect(onChange).toHaveBeenCalledWith([{ name: 'OpenRouter', baseURL: 'https://old' }]); + }); + + it('keeps a headers container the admin actually edited (array-shaped, even if emptied)', () => { + const onChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByText('com_ui_add_item')); + expect(onChange).toHaveBeenCalledWith([ + {}, + { name: 'OpenRouter', baseURL: 'https://old', headers: [] }, + { name: 'Anyscale', baseURL: 'https://anyscale' }, + ]); + }); +}); diff --git a/src/components/configuration/fields/ArrayObjectField.tsx b/src/components/configuration/fields/ArrayObjectField.tsx index 52b807cb..b768aa51 100644 --- a/src/components/configuration/fields/ArrayObjectField.tsx +++ b/src/components/configuration/fields/ArrayObjectField.tsx @@ -1,5 +1,10 @@ import { useCallback, useState, useRef, useEffect } from 'react'; import type * as t from '@/types'; +import { + PREVIOUS_IDENTITY_HINT_KEY, + withPreviousIdentityHint, + stripUntouchedSecretRecordContainers, +} from '@/utils'; import { ObjectEntryCard } from './ObjectEntryCard'; import { AddItemButton } from '@/components/shared'; import { useLocalize } from '@/hooks'; @@ -14,6 +19,10 @@ function getEntryLabel(item: t.ConfigValue): string | null { return null; } +function nonEmptyString(value: t.ConfigValue): string | undefined { + return typeof value === 'string' && value !== '' ? value : undefined; +} + export function ArrayObjectField({ id, value, @@ -26,6 +35,7 @@ export function ArrayObjectField({ renderFields, entryIdPrefix, editSessionId, + identityKey, }: t.ArrayObjectFieldProps) { const localize = useLocalize(); const items = Array.isArray(value) ? (value as t.ConfigValue[]) : []; @@ -43,6 +53,30 @@ export function ArrayObjectField({ // already prepended the key; we wait for the parent's items to catch up). const addingRef = useRef(false); + // Records each entry's identity value (`name`/`group`) the first time it is + // edited this session, so a later rename in the same edit can still be + // traced back to it. `null` is a distinct, explicit value here — see + // `withPreviousIdentityHint`'s doc comment. Captured lazily inside + // `handleEntryChange` rather than unconditionally during render: a plain + // object (not React state) mutated during render is read by React + // internals across the current and work-in-progress trees, so a render + // that gets interrupted or retried with different props could leave it + // holding a value from a render that never committed. An event handler has + // no such race — it only ever runs against the props/state that actually + // committed. + // + // A save, reset, restore, discard, scope change, or conflict rebase all + // bump `editSessionId` — the array below it is now a fresh baseline, not a + // continuation of what was open before, so this cache (keyed by a stable + // key that itself resets on remount) must not survive across the boundary. + // Rather than clear it with a render-time ref write — safe for the + // documented `setState`-during-render pattern, but refs have no such + // guarantee under an interrupted/retried render — the call sites key + // `` on this same id, so React fully + // remounts the component (fresh refs and state, no manual reset needed) + // exactly when a session boundary is crossed. + const originalIdentityRef = useRef>(new Map()); + // Sync keys array length with items (handles external changes like // save/re-fetch). Skipped right after handleAdd since keys were already // prepended locally and items will arrive next render. @@ -62,8 +96,20 @@ export function ArrayObjectField({ addingRef.current = true; setKeys((prev) => [newKey, ...prev]); expandedKeyRef.current = newKey; - onChange([{}, ...items]); - }, [items, onChange]); + // Stamped explicitly, not left absent: an absent hint falls back to + // bare-identity matching, which would let this new entry inherit an + // existing entry's credentials merely by being given the same name + // later in this same session (e.g. reusing a name just freed by + // deleting that other entry) — see `withPreviousIdentityHint`. + const blank: t.ConfigValue = identityKey ? { [PREVIOUS_IDENTITY_HINT_KEY]: null } : {}; + // Surviving entries are copied forward verbatim by this structural add — + // strip any redacted credential-record placeholder left on them by a + // read, or resubmitting it here would erase that entry's real secret. + onChange([ + blank, + ...items.map((item) => stripUntouchedSecretRecordContainers(item, fields)), + ]); + }, [items, onChange, identityKey, fields]); // Expose add trigger to parent (e.g. NestedGroup / section header button) useEffect(() => { @@ -78,22 +124,68 @@ export function ArrayObjectField({ const handleRemove = useCallback( (index: number) => { setKeys((prev) => prev.filter((_, i) => i !== index)); - onChange(items.filter((_, i) => i !== index)); + onChange( + items + .filter((_, i) => i !== index) + .map((item) => stripUntouchedSecretRecordContainers(item, fields)), + ); }, - [items, onChange], + [items, onChange, fields], ); const handleEntryChange = useCallback( (index: number, newValue: t.ConfigValue) => { + let hinted = newValue; + if (identityKey) { + const stableKey = keys[index]; + if (stableKey != null && !originalIdentityRef.current.has(stableKey)) { + const currentItem = items[index]; + const currentRecord = + currentItem && typeof currentItem === 'object' && !Array.isArray(currentItem) + ? (currentItem as Record) + : undefined; + // A remount (e.g. switching tabs and back) within the same edit + // session throws away this component instance's origin map, but + // `editedValues` — and so `currentItem` — is unaffected: if an + // earlier edit already attached a hint (a rename's real origin, or + // the explicit "no origin" marker from creation), it's still + // sitting right there on the entry. Prefer it over the entry's + // current identity, or a fresh mount would "recapture" the + // already-renamed value (or manufacture an origin for a brand-new + // entry) as if it were the true origin, permanently losing the + // real signal. + // + // An entry with neither an embedded hint nor an existing identity + // (a pre-existing stored row saved blank, or a row edited before + // ever being named) caches `null`, not "leave uncached": once this + // entry IS given a name later in the same edit, an uncached slot + // would fall through to treating that brand-new name as if it were + // this entry's own long-standing identity — the same ambiguity a + // stamped-null brand-new entry closes, just reached by editing an + // existing identity-less row instead of adding a new one. `get` + // returning `undefined` after this point means only "capture + // hasn't run yet," never "there's genuinely nothing to restore." + const embeddedHint = currentRecord?.[PREVIOUS_IDENTITY_HINT_KEY]; + const origin = + embeddedHint === null + ? null + : nonEmptyString(embeddedHint) ?? nonEmptyString(currentRecord?.[identityKey]) ?? null; + originalIdentityRef.current.set(stableKey, origin); + } + hinted = withPreviousIdentityHint( + newValue, + stableKey != null ? originalIdentityRef.current.get(stableKey) : undefined, + ); + } if (onEntryChange) { - onEntryChange(index, newValue); + onEntryChange(index, hinted); return; } const next = [...items]; - next[index] = newValue; + next[index] = hinted; onChange(next); }, - [items, onChange, onEntryChange], + [items, onChange, onEntryChange, identityKey, keys], ); return ( diff --git a/src/components/configuration/fields/KeyValueField.test.tsx b/src/components/configuration/fields/KeyValueField.test.tsx new file mode 100644 index 00000000..53f1cd8e --- /dev/null +++ b/src/components/configuration/fields/KeyValueField.test.tsx @@ -0,0 +1,140 @@ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { KeyValueField } from './KeyValueField'; + +vi.mock('@/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +vi.mock('@clickhouse/click-ui', () => { + function Select({ + children, + value, + onSelect, + 'aria-label': ariaLabel, + }: { + children?: React.ReactNode; + value?: string; + onSelect?: (value: string) => void; + 'aria-label'?: string; + }) { + return ( + + ); + } + Select.Item = ({ value, children }: { value: string; children?: React.ReactNode }) => ( + + ); + return { + Select, + IconButton: ({ + onClick, + 'aria-label': ariaLabel, + }: { + onClick?: () => void; + 'aria-label'?: string; + }) => + ), + }; +}); + +describe('KeyValueField — reserved __previousIdentity key', () => { + it('flags __previousIdentity as reserved on an mcpServers headers path, without dropping the row', () => { + render( + , + ); + expect(screen.getByText('com_config_header_key_reserved')).toBeInTheDocument(); + expect(screen.getByLabelText('com_ui_key 1')).toHaveClass('config-input-error'); + }); + + it('flags __previousIdentity as reserved on an mcpServers oauth_headers path too', () => { + render( + , + ); + expect(screen.getByText('com_config_header_key_reserved')).toBeInTheDocument(); + }); + + it('does not flag __previousIdentity on an unrelated field path — no hint protocol to collide with', () => { + render( + , + ); + expect(screen.queryByText('com_config_header_key_reserved')).not.toBeInTheDocument(); + expect(screen.getByLabelText('com_ui_key 1')).not.toHaveClass('config-input-error'); + }); + + it('does not flag __previousIdentity when no fieldPath is supplied at all', () => { + render( + , + ); + expect(screen.queryByText('com_config_header_key_reserved')).not.toBeInTheDocument(); + }); + + it('does not flag a normal header key on an mcpServers headers path', () => { + render( + , + ); + expect(screen.queryByText('com_config_header_key_reserved')).not.toBeInTheDocument(); + }); + + it('flags __previousIdentity as reserved on a json-typed row too', () => { + render( + , + ); + expect(screen.getByText('com_config_header_key_reserved')).toBeInTheDocument(); + }); + + it('still calls onChange with the row intact when the reserved key is typed — the drop happens at save-serialization, not here', () => { + const onChange = vi.fn(); + render( + , + ); + fireEvent.blur(screen.getByLabelText('com_ui_key 1'), { + target: { value: '__previousIdentity' }, + }); + fireEvent.change(screen.getByLabelText('com_ui_key 1'), { + target: { value: '__previousIdentity' }, + }); + fireEvent.blur(screen.getByLabelText('com_ui_key 1')); + expect(onChange).toHaveBeenCalledWith([ + { key: '__previousIdentity', value: '', valueType: 'string' }, + ]); + }); +}); diff --git a/src/components/configuration/fields/KeyValueField.tsx b/src/components/configuration/fields/KeyValueField.tsx index 965aaeaf..8cb98d1c 100644 --- a/src/components/configuration/fields/KeyValueField.tsx +++ b/src/components/configuration/fields/KeyValueField.tsx @@ -2,6 +2,7 @@ import { Select } from '@clickhouse/click-ui'; import TextareaAutosize from 'react-textarea-autosize'; import { useState, useEffect, useRef, useLayoutEffect } from 'react'; import type * as t from '@/types'; +import { cn, isMcpServerHeadersContainerPath, PREVIOUS_IDENTITY_HINT_KEY } from '@/utils'; import { AddItemButton, TrashButton } from '@/components/shared'; import { useLocalize } from '@/hooks'; @@ -115,11 +116,21 @@ export function KeyValueField({ keyPlaceholder, valuePlaceholder, 'aria-label': ariaLabel, + fieldPath, }: t.KeyValueFieldProps) { const localize = useLocalize(); const availableTypes = valueTypes ?? DEFAULT_TYPES; const listRef = useRef(null); const focusLastKeyRef = useRef(false); + /** + * `__previousIdentity` is the mcpServers rename/create origin-hint key + * (see `withPreviousIdentityHint`) — it only collides with an admin-typed + * key on the exact headers/oauth_headers container that hint protocol + * rides on; every other KeyValueField-backed record field has no such + * collision, so the reserved name is only flagged here. + */ + const isReservedKeyScope = fieldPath != null && isMcpServerHeadersContainerPath(fieldPath); + const isReservedKey = (key: string) => isReservedKeyScope && key === PREVIOUS_IDENTITY_HINT_KEY; useLayoutEffect(() => { if (focusLastKeyRef.current) { @@ -153,65 +164,75 @@ export function KeyValueField({ const renderPrimitiveRow = (vType: t.KVValueType, pair: t.KeyValuePair, index: number) => { const valueLabel = `${localize('com_ui_value')} ${index + 1}`; + const reserved = isReservedKey(pair.key); return ( -
- handleChange(index, 'key', v)} - placeholder={keyPlaceholder ?? localize('com_ui_key')} - disabled={disabled} - aria-label={`${localize('com_ui_key')} ${index + 1}`} - className="config-input max-w-37.5 flex-1" - /> - {vType === 'boolean' ? ( -
- -
- ) : ( +
+
handleChange(index, 'value', v)} - placeholder={valuePlaceholder ?? localize('com_ui_value')} + value={pair.key} + onCommit={(v) => handleChange(index, 'key', v)} + placeholder={keyPlaceholder ?? localize('com_ui_key')} disabled={disabled} - aria-label={valueLabel} - className="config-input flex-2" - /> - )} - {!disabled && availableTypes.length > 1 && ( -
- -
- )} - {!disabled && ( - handleRemove(index)} - ariaLabel={`${localize('com_ui_delete')} ${localize('com_ui_entry')} ${index + 1}`} + aria-label={`${localize('com_ui_key')} ${index + 1}`} + className={cn('config-input max-w-37.5 flex-1', reserved && 'config-input-error')} /> + {vType === 'boolean' ? ( +
+ +
+ ) : ( + handleChange(index, 'value', v)} + placeholder={valuePlaceholder ?? localize('com_ui_value')} + disabled={disabled} + aria-label={valueLabel} + className="config-input flex-2" + /> + )} + {!disabled && availableTypes.length > 1 && ( +
+ +
+ )} + {!disabled && ( + handleRemove(index)} + ariaLabel={`${localize('com_ui_delete')} ${localize('com_ui_entry')} ${index + 1}`} + /> + )} +
+ {reserved && ( + + {localize('com_config_header_key_reserved')} + )}
); }; - const renderJsonRow = (pair: t.KeyValuePair, index: number) => ( + const renderJsonRow = (pair: t.KeyValuePair, index: number) => { + const reserved = isReservedKey(pair.key); + return (
{!disabled && availableTypes.length > 1 && (
@@ -251,8 +272,14 @@ export function KeyValueField({ disabled={disabled} aria-label={`${localize('com_ui_value')} ${index + 1}`} /> + {reserved && ( + + {localize('com_config_header_key_reserved')} + + )}
- ); + ); + }; return (
({ + useLocalize: () => (key: string) => key, +})); + +vi.mock('@clickhouse/click-ui', () => ({ + Icon: () => null, + IconButton: ({ + onClick, + 'aria-label': ariaLabel, + }: { + onClick?: () => void; + 'aria-label'?: string; + }) => + ); +} + +describe('ObjectEntryCard — credential-record container preservation', () => { + it('omits an untouched credential-record container when an unrelated field is edited', () => { + const onValueChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByText('trigger')); + expect(onValueChange).toHaveBeenCalledTimes(1); + const submitted = onValueChange.mock.calls[0][0] as Record; + expect(submitted).toEqual({ name: 'OpenRouter', baseURL: 'https://new' }); + expect(Object.hasOwn(submitted, 'headers')).toBe(false); + }); + + it('keeps a credential-record container the admin actually edited, even once emptied', () => { + const onValueChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByText('trigger')); + const submitted = onValueChange.mock.calls[0][0] as Record; + expect(submitted).toEqual({ name: 'OpenRouter', baseURL: 'https://old', headers: [] }); + }); + + it('does not mask a record field that is not a registered credential container', () => { + const fieldsWithPlainRecord = [ + ...entryFields, + createField({ key: 'metadata', type: 'record', recordValueType: 'primitive' }), + ]; + const onValueChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByText('trigger')); + const submitted = onValueChange.mock.calls[0][0] as Record; + expect(submitted).toEqual({ + name: 'OpenRouter', + baseURL: 'https://new', + metadata: { a: 'b' }, + }); + }); + + it('stays omitted across a further unrelated edit once a container is already absent', () => { + const onValueChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByText('trigger')); + const submitted = onValueChange.mock.calls[0][0] as Record; + expect(submitted).toEqual({ name: 'Renamed', baseURL: 'https://old' }); + expect(Object.hasOwn(submitted, 'headers')).toBe(false); + }); +}); diff --git a/src/components/configuration/fields/ObjectEntryCard.tsx b/src/components/configuration/fields/ObjectEntryCard.tsx index 4e4a3121..48c2b7cb 100644 --- a/src/components/configuration/fields/ObjectEntryCard.tsx +++ b/src/components/configuration/fields/ObjectEntryCard.tsx @@ -1,10 +1,10 @@ import { Icon } from '@clickhouse/click-ui'; import { useState, useCallback, useRef, useEffect } from 'react'; import type * as t from '@/types'; +import { cn, stripUntouchedSecretRecordContainers } from '@/utils'; import { TrashButton } from '@/components/shared'; import { CodeField } from './CodeField'; import { useLocalize } from '@/hooks'; -import { cn } from '@/utils'; export function ObjectEntryCard({ id, @@ -69,9 +69,10 @@ export function ObjectEntryCard({ : {}; const segments = fieldPath.split('.'); const leafKey = segments[segments.length - 1]; - onValueChange({ ...current, [leafKey]: fieldValue }); + const next: Record = { ...current, [leafKey]: fieldValue }; + onValueChange(stripUntouchedSecretRecordContainers(next, fields)); }, - [value, onValueChange], + [value, onValueChange, fields], ); const commitRename = useCallback(() => { diff --git a/src/components/configuration/fields/RecordObjectField.test.tsx b/src/components/configuration/fields/RecordObjectField.test.tsx new file mode 100644 index 00000000..0d115812 --- /dev/null +++ b/src/components/configuration/fields/RecordObjectField.test.tsx @@ -0,0 +1,116 @@ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import type * as t from '@/types'; +import { RecordObjectField } from './RecordObjectField'; +import { createField } from '@/test/fixtures'; + +vi.mock('@/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +vi.mock('@clickhouse/click-ui', () => ({ + Icon: () => null, + IconButton: ({ + onClick, + 'aria-label': ariaLabel, + }: { + onClick?: () => void; + 'aria-label'?: string; + }) => + ), +})); + +const entryFields: t.SchemaField[] = [ + createField({ key: 'baseURL', type: 'string' }), + createField({ key: 'headers', type: 'record', recordValueType: 'primitive' }), +]; + +const noopRenderFields: t.CollectionRenderFields = () => null; + +function expandEntry(label: string) { + fireEvent.click(screen.getByText(label).closest('[role="button"]') as HTMLElement); +} + +describe('RecordObjectField — strips untouched secret-record containers on structural edits', () => { + it('strips a redacted headers placeholder from a surviving entry when adding a new key', () => { + const onChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByText('com_ui_add_item')); + fireEvent.change(screen.getByPlaceholderText('com_ui_key'), { target: { value: 'newKey' } }); + fireEvent.click(screen.getByText('com_ui_add')); + expect(onChange).toHaveBeenCalledWith({ + newKey: {}, + existing: { baseURL: 'https://old' }, + }); + }); + + it('strips a redacted headers placeholder from a surviving entry when removing another', () => { + const onChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getAllByLabelText(/com_ui_delete/)[1]); + expect(onChange).toHaveBeenCalledWith({ a: { baseURL: 'https://old' } }); + }); + + it('strips a redacted headers placeholder from a surviving entry when renaming another', () => { + const onChange = vi.fn(); + render( + , + ); + expandEntry('b'); + fireEvent.click(screen.getByLabelText(/com_a11y_rename_entry/)); + fireEvent.change(screen.getByDisplayValue('b'), { target: { value: 'renamed' } }); + fireEvent.keyDown(screen.getByDisplayValue('renamed'), { key: 'Enter' }); + expect(onChange).toHaveBeenCalledWith({ + a: { baseURL: 'https://old' }, + renamed: { baseURL: 'https://other' }, + }); + }); + + it('keeps a headers container the admin actually edited (array-shaped, even if emptied)', () => { + const onChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getAllByLabelText(/com_ui_delete/)[1]); + expect(onChange).toHaveBeenCalledWith({ a: { baseURL: 'https://old', headers: [] } }); + }); +}); diff --git a/src/components/configuration/fields/RecordObjectField.tsx b/src/components/configuration/fields/RecordObjectField.tsx index 3c51b6d0..93763f7e 100644 --- a/src/components/configuration/fields/RecordObjectField.tsx +++ b/src/components/configuration/fields/RecordObjectField.tsx @@ -1,6 +1,7 @@ import { Button } from '@clickhouse/click-ui'; import { useState, useCallback, useEffect, memo } from 'react'; import type * as t from '@/types'; +import { stripUntouchedSecretRecordContainers } from '@/utils'; import { ObjectEntryCard } from './ObjectEntryCard'; import { AddItemButton } from '@/components/shared'; import { useLocalize } from '@/hooks'; @@ -39,27 +40,33 @@ export function RecordObjectField({ const handleAdd = useCallback( (key: string) => { if (key in record) return; - // Prepend: new key first, then existing entries + // Prepend: new key first, then existing entries. Surviving entries are + // copied forward verbatim here — strip any redacted credential-record + // placeholder left on them by a read, or resubmitting it would erase + // that entry's real secret. const next: Record = { [key]: allowPrimitiveValues ? true : {}, }; for (const [k, v] of Object.entries(record)) { - next[k] = v; + next[k] = stripUntouchedSecretRecordContainers(v, fields); } onChange(next); setJustAddedKey(key); setShowAddInput(false); }, - [record, onChange, allowPrimitiveValues], + [record, onChange, allowPrimitiveValues, fields], ); const handleRemove = useCallback( (key: string) => { - const next = { ...record }; - delete next[key]; + const next: Record = {}; + for (const [k, v] of Object.entries(record)) { + if (k === key) continue; + next[k] = stripUntouchedSecretRecordContainers(v, fields); + } onChange(next); }, - [record, onChange], + [record, onChange, fields], ); const handleEntryChange = useCallback( @@ -74,11 +81,11 @@ export function RecordObjectField({ if (renamed === oldKey || renamed in record) return; const next: Record = {}; for (const [k, v] of Object.entries(record)) { - next[k === oldKey ? renamed : k] = v; + next[k === oldKey ? renamed : k] = stripUntouchedSecretRecordContainers(v, fields); } onChange(next); }, - [record, onChange], + [record, onChange, fields], ); return ( diff --git a/src/components/configuration/index.ts b/src/components/configuration/index.ts index 6afed19e..b6922c2d 100644 --- a/src/components/configuration/index.ts +++ b/src/components/configuration/index.ts @@ -7,6 +7,7 @@ export { ConfigTabBar } from './ConfigTabBar'; export { ConfigTabContent } from './ConfigTabContent'; export { ConfigPage } from './ConfigPage'; export { ImportYamlDialog } from './ImportYamlDialog'; +export { RevisionHistoryDialog } from './RevisionHistoryDialog'; export { InfoBanner } from './InfoBanner'; export { getControlType, getEnumOptions, getArrayItemType } from './utils'; diff --git a/src/components/configuration/queries.ts b/src/components/configuration/queries.ts new file mode 100644 index 00000000..c3bbc24f --- /dev/null +++ b/src/components/configuration/queries.ts @@ -0,0 +1,17 @@ +import type { QueryClient } from '@tanstack/react-query'; +import { baseConfigOptions, getBaseConfigFn } from '@/server'; +import { installIfNewer } from './utils'; + +/** A direct read cannot join a stale in-flight fetch. Cancel before reading; + * compare versions afterward because independent direct reads can still race. */ +export async function refreshBaseConfig(queryClient: QueryClient) { + await queryClient.cancelQueries({ queryKey: baseConfigOptions.queryKey }); + const fresh = await getBaseConfigFn(); + return installIfNewer( + queryClient, + [...baseConfigOptions.queryKey, fresh.effectiveTenantId], + fresh, + (value) => value.dbConfigVersion, + (value) => value.effectiveTenantId, + ); +} diff --git a/src/components/configuration/sections/CreateCustomEndpointDialog.test.tsx b/src/components/configuration/sections/CreateCustomEndpointDialog.test.tsx new file mode 100644 index 00000000..2a51306b --- /dev/null +++ b/src/components/configuration/sections/CreateCustomEndpointDialog.test.tsx @@ -0,0 +1,96 @@ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import type * as t from '@/types'; +import { CreateCustomEndpointDialog } from './CreateCustomEndpointDialog'; +import { createField } from '@/test/fixtures'; + +vi.mock('@/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +vi.mock('@clickhouse/click-ui', () => ({ + Button: ({ + label, + onClick, + disabled, + type, + }: { + label?: string; + onClick?: () => void; + disabled?: boolean; + type?: string; + }) => ( + + ), + Dialog: Object.assign( + ({ open, children }: { open: boolean; children: React.ReactNode }) => + open ?
{children}
: null, + { + Content: ({ title, children }: { title: string; children: React.ReactNode }) => ( + <> +

{title}

+ {children} + + ), + }, + ), +})); + +const nameField: t.SchemaField[] = [createField({ key: 'name', type: 'string' })]; + +const renderNameInput: t.CollectionRenderFields = (_fields, parentValue, _entryKey, onChange) => { + const value = + parentValue && typeof parentValue === 'object' && !Array.isArray(parentValue) + ? ((parentValue as Record).name as string | undefined) + : undefined; + return ( + onChange('name', e.target.value)} + /> + ); +}; + +describe('CreateCustomEndpointDialog — duplicate name prevention', () => { + it('rejects a name that already exists instead of calling onSave', () => { + const onSave = vi.fn(); + render( + , + ); + fireEvent.change(screen.getByLabelText('name'), { target: { value: 'OpenRouter' } }); + fireEvent.click(screen.getByText('com_ui_create')); + + expect(onSave).not.toHaveBeenCalled(); + expect(screen.getByRole('alert')).toHaveTextContent('com_config_endpoint_name_duplicate'); + }); + + it('saves and closes for a name that does not collide with an existing entry', () => { + const onSave = vi.fn(); + const onClose = vi.fn(); + render( + , + ); + fireEvent.change(screen.getByLabelText('name'), { target: { value: 'Anyscale' } }); + fireEvent.click(screen.getByText('com_ui_create')); + + expect(onSave).toHaveBeenCalledWith({ name: 'Anyscale', __previousIdentity: null }); + expect(onClose).toHaveBeenCalled(); + }); +}); diff --git a/src/components/configuration/sections/CreateCustomEndpointDialog.tsx b/src/components/configuration/sections/CreateCustomEndpointDialog.tsx index ad20ce3a..67c017a5 100644 --- a/src/components/configuration/sections/CreateCustomEndpointDialog.tsx +++ b/src/components/configuration/sections/CreateCustomEndpointDialog.tsx @@ -8,6 +8,7 @@ import { useState, useCallback } from 'react'; import type * as t from '@/types'; +import { PREVIOUS_IDENTITY_HINT_KEY } from '@/utils'; import { FormDialog } from '@/components/shared'; import { useLocalize } from '@/hooks'; @@ -17,12 +18,19 @@ export function CreateCustomEndpointDialog({ onSave, fields, renderFields, + existingNames, }: { open: boolean; onClose: () => void; onSave: (entry: Record) => void; fields: t.SchemaField[]; renderFields: t.CollectionRenderFields; + /** Names already in use — the backend keys credential preservation and + * restoration by this exact identity, and rejects a same-request + * collision outright rather than guessing which entry the credentials + * belong to; this check gives that feedback immediately instead of + * round-tripping to the server first. */ + existingNames: ReadonlySet; }) { const localize = useLocalize(); const [draft, setDraft] = useState>({}); @@ -39,7 +47,17 @@ export function CreateCustomEndpointDialog({ setError(localize('com_config_endpoint_name_required')); return; } - const entry: Record = {}; + if (existingNames.has(name)) { + setError(localize('com_config_endpoint_name_duplicate')); + return; + } + const entry: Record = { + // Explicit, not absent: an absent hint falls back to bare-identity + // matching on save, which would let this brand-new entry inherit + // another entry's credentials merely by reusing a name freed up by a + // delete earlier in the same edit — see `withPreviousIdentityHint`. + [PREVIOUS_IDENTITY_HINT_KEY]: null, + }; for (const [key, val] of Object.entries(draft)) { if (val === '' || val === undefined || val === null) continue; if (Array.isArray(val) && val.length === 0) continue; @@ -49,7 +67,7 @@ export function CreateCustomEndpointDialog({ setDraft({}); setError(undefined); onClose(); - }, [draft, localize, onSave, onClose]); + }, [draft, localize, onSave, onClose, existingNames]); const handleClose = useCallback(() => { setDraft({}); diff --git a/src/components/configuration/sections/EndpointsRenderer.tsx b/src/components/configuration/sections/EndpointsRenderer.tsx index d18c1806..205f4736 100644 --- a/src/components/configuration/sections/EndpointsRenderer.tsx +++ b/src/components/configuration/sections/EndpointsRenderer.tsx @@ -20,12 +20,12 @@ import type { ReactNode } from 'react'; import type * as t from '@/types'; import { FieldRenderer, NestedGroup, renderInlineField } from '../FieldRenderer'; import { CreateCustomEndpointDialog } from './CreateCustomEndpointDialog'; +import { cn, stripUntouchedSecretRecordContainers } from '@/utils'; import { useCollapsibleSection } from '../useCollapsibleSection'; import { ArrayObjectField } from '../fields/ArrayObjectField'; import { countConfigured, hasDescendant } from '../utils'; import { renderCollapsible } from '../renderCollapsible'; import { useLocalize } from '@/hooks'; -import { cn } from '@/utils'; // --------------------------------------------------------------------------- // Constants @@ -629,9 +629,24 @@ export function CustomEndpointsRenderer(props: t.FieldRendererProps) { : {}; const value = getValue(path, parentObj[customField.key] ?? []); const items = Array.isArray(value) ? value : []; + const existingNames = new Set( + items + .map((item) => + item && typeof item === 'object' && !Array.isArray(item) + ? (item as Record).name + : undefined, + ) + .filter((name): name is string => typeof name === 'string' && name !== ''), + ); const handleCreate = (entry: Record) => { - onChange(path, [...items, entry]); + // Surviving entries are copied forward verbatim by this structural add — + // strip any redacted credential-record placeholder left on them by a + // read, or resubmitting it here would erase that entry's real secret. + const strippedItems = items.map((item) => + stripUntouchedSecretRecordContainers(item, customField.children ?? []), + ); + onChange(path, [...strippedItems, entry]); }; const isEmpty = items.length === 0; @@ -652,6 +667,10 @@ export function CustomEndpointsRenderer(props: t.FieldRendererProps) {
) : ( )}
); diff --git a/src/components/configuration/sections/LangfuseRenderer.tsx b/src/components/configuration/sections/LangfuseRenderer.tsx index 6bf966aa..ade1b855 100644 --- a/src/components/configuration/sections/LangfuseRenderer.tsx +++ b/src/components/configuration/sections/LangfuseRenderer.tsx @@ -1,18 +1,23 @@ -import { useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { Button, Select, TextField } from '@clickhouse/click-ui'; import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; -import type * as t from '@/types'; import type { LangfuseConnectionStatus } from '@/server'; +import type * as t from '@/types'; import { + baseConfigOptions, getLangfuseConnectionFn, LANGFUSE_CONNECTION_QUERY_KEY, testLangfuseConnectionFn, updateLangfuseConnectionFn, } from '@/server'; +import { installIfNewer, versionedStructuralSharing } from '../utils'; +import { VersionConflictDialog } from '../VersionConflictDialog'; +import { isVersionConflictError } from '@/server/utils/errors'; import { notifyError, notifySuccess } from '@/utils'; -import { useLocalize } from '@/hooks'; +import { useLocalize, useConfigSession } from '@/hooks'; +import { refreshBaseConfig } from '../queries'; -type VerificationState = 'idle' | 'unverified' | 'checking' | 'verified' | 'failed'; +type VerificationState = 'idle' | 'inactive' | 'unverified' | 'checking' | 'verified' | 'failed'; function getConnectionKey(status?: LangfuseConnectionStatus): string | undefined { if (!status?.configured || !status.destination || !status.publicKey) return undefined; @@ -37,6 +42,8 @@ function getVerificationLabel( return localize('com_config_langfuse_verified'); case 'failed': return message || localize('com_config_langfuse_test_fail'); + case 'inactive': + return localize('com_config_langfuse_inactive'); case 'unverified': return localize('com_config_langfuse_not_verified'); default: @@ -57,52 +64,175 @@ function getVerificationDotClass(state: VerificationState): string { } } -export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererProps) { +export function LangfuseRenderer({ + disabled, + isEditingScope, + effectiveTenantId, +}: t.FieldRendererProps) { const localize = useLocalize(); const queryClient = useQueryClient(); const [status, setStatus] = useState(); - const [destination, setDestination] = useState(''); - const [publicKey, setPublicKey] = useState(''); - const [secretKey, setSecretKey] = useState(''); + const { + baseline: { version: expectedVersion, tenantId: expectedTenantId }, + adoptBaseline, + draft: { destination, publicKey, secretKey }, + setDraft, + conflictOpen: versionConflictOpen, + setConflictOpen: setVersionConflictOpen, + resolveConflict, + rebasing: rebasingVersion, + discarding: discardingConflict, + } = useConfigSession( + { version: null, tenantId: effectiveTenantId ?? '', value: undefined }, + { destination: '', publicKey: '', secretKey: '' }, + ); + const setDestination = useCallback( + (value: string) => setDraft((draft) => ({ ...draft, destination: value })), + [setDraft], + ); + const setPublicKey = useCallback( + (value: string) => setDraft((draft) => ({ ...draft, publicKey: value })), + [setDraft], + ); + const setSecretKey = useCallback( + (value: string) => setDraft((draft) => ({ ...draft, secretKey: value })), + [setDraft], + ); const [editingPublicKey, setEditingPublicKey] = useState(false); const [editingSecretKey, setEditingSecretKey] = useState(false); const [verificationState, setVerificationState] = useState('idle'); const [verificationMessage, setVerificationMessage] = useState(''); const testedConnectionRef = useRef(undefined); const requestRef = useRef(0); - const hasDraftRef = useRef(false); + /** + * Whether this field's current value differs from `status` right now — + * tracked per field, not as one combined flag, so editing only the secret + * doesn't also freeze destination/public key against a concurrent change to + * them. A background sync (the effect below) or a conflict rebase must not + * clobber a real divergence with the refetched value; only fields that + * still match get the fresh baseline. + */ + const destinationTouchedRef = useRef(false); + const publicKeyTouchedRef = useRef(false); + /** Same "current dirtiness" role as the refs above, but for the secret key + * draft — tracked via a ref instead of reading `secretKey` state directly + * inside the sync effect below, so that effect stays free of a dependency + * that would otherwise fire it on every keystroke. There's no baseline to + * compare against (the server never sends back a real secret), so any + * non-empty draft counts as dirty. */ + const secretKeyDraftRef = useRef(false); + const [tenantScope, setTenantScope] = useState(effectiveTenantId ?? ''); + /** + * The highest `configVersion` this component has adopted so far, from any + * source. The query cache now rejects older tracked results through + * `versionedStructuralSharing`; this local guard is still required at the + * component boundary so a pre-existing/dehydrated cache entry or another + * caller without that policy cannot regress displayed fields while a draft + * keeps `expectedVersion` frozen at the newer version. + */ + const latestVersionRef = useRef(null); + const latestTenantRef = useRef(effectiveTenantId ?? ''); + const hasAdoptedStatusRef = useRef(false); + const connectionQueryKey = useMemo( + () => + tenantScope + ? ([...LANGFUSE_CONNECTION_QUERY_KEY, tenantScope] as const) + : LANGFUSE_CONNECTION_QUERY_KEY, + [tenantScope], + ); const connectionQuery = useQuery({ - queryKey: LANGFUSE_CONNECTION_QUERY_KEY, - queryFn: () => getLangfuseConnectionFn(), - enabled: !isEditingScope, + queryKey: connectionQueryKey, + queryFn: () => getLangfuseConnectionFn({ data: { expectedTenantId: tenantScope } }), + structuralSharing: versionedStructuralSharing( + (value) => value.configVersion, + (value) => value.effectiveTenantId ?? tenantScope, + ), + enabled: !isEditingScope && effectiveTenantId !== undefined, retry: false, + refetchOnMount: 'always', refetchOnWindowFocus: false, }); + + useEffect(() => { + if (effectiveTenantId === undefined || effectiveTenantId === tenantScope) return; + setTenantScope(effectiveTenantId); + }, [effectiveTenantId, tenantScope]); const updateMutation = useMutation({ mutationFn: (data: { enabled: boolean; destination: string; publicKey: string; secretKey?: string; + expectedVersion: number | null; + expectedTenantId: string; }) => updateLangfuseConnectionFn({ data }), }); const testMutation = useMutation({ - mutationFn: (data: { destination: string; publicKey: string; secretKey?: string }) => - testLangfuseConnectionFn({ data }), + mutationFn: (data: { + destination: string; + publicKey: string; + secretKey?: string; + expectedTenantId: string; + }) => testLangfuseConnectionFn({ data }), }); + /** + * Whether `candidate` is older than the highest version this component has + * already adopted — see `latestVersionRef`'s doc comment. A numeric ref + * always outranks a null candidate version: once a real version has been + * adopted, a candidate with no version at all is older by definition. + */ + const isStaleStatus = (candidate: LangfuseConnectionStatus): boolean => + (candidate.effectiveTenantId ?? latestTenantRef.current) === latestTenantRef.current && + latestVersionRef.current != null && + (candidate.configVersion == null || candidate.configVersion < latestVersionRef.current); + useEffect(() => { - if (!connectionQuery.data) return; - if (hasDraftRef.current) return; + if (!connectionQuery.data || isStaleStatus(connectionQuery.data)) return; const nextStatus = connectionQuery.data; + const nextTenantId = nextStatus.effectiveTenantId ?? latestTenantRef.current; + const tenantChanged = hasAdoptedStatusRef.current && nextTenantId !== latestTenantRef.current; + if (tenantChanged) { + destinationTouchedRef.current = false; + publicKeyTouchedRef.current = false; + secretKeyDraftRef.current = false; + setSecretKey(''); + setEditingPublicKey(false); + setEditingSecretKey(false); + setVersionConflictOpen(false); + notifyError(localize('com_config_tenant_changed')); + } + latestTenantRef.current = nextTenantId; + hasAdoptedStatusRef.current = true; + latestVersionRef.current = nextStatus.configVersion; + setTenantScope(nextTenantId); setStatus(nextStatus); // Preserve the stored destination for display even when the server dropped it from the // allowlist. Blanking it made destinationChanged true, forcing edit mode and leaving an // enabled connection impossible to disable until a replacement was picked; a de-allowlisted // destination now simply shows as unselected in the picker while disable stays available. - setDestination(nextStatus.destination ?? ''); - setPublicKey(nextStatus.publicKey ?? ''); + if (!destinationTouchedRef.current) { + setDestination(nextStatus.destination ?? ''); + } else if (destination === (nextStatus.destination ?? '')) { + destinationTouchedRef.current = false; + } + if (!publicKeyTouchedRef.current) { + setPublicKey(nextStatus.publicKey ?? ''); + } else if (publicKey.trim() === (nextStatus.publicKey ?? '')) { + publicKeyTouchedRef.current = false; + } + // A passive sync must not advance the CAS token while a draft survives — + // see the `expectedVersion` docstring above. + const hasLocalDraft = + destinationTouchedRef.current || publicKeyTouchedRef.current || secretKeyDraftRef.current; + if (!hasLocalDraft) { + adoptBaseline({ + version: nextStatus.configVersion ?? null, + tenantId: nextTenantId, + value: nextStatus, + }); + } }, [connectionQuery.data]); useEffect(() => { @@ -112,6 +242,13 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr setVerificationMessage(''); return; } + if (status?.configActive === false) { + requestRef.current += 1; + testedConnectionRef.current = undefined; + setVerificationState('inactive'); + setVerificationMessage(''); + return; + } // Read-only viewers lack manage:configs:langfuse and cannot run verification. Show the stored // connection as unverified rather than "not configured", and clear the in-flight and // tested-connection markers so switching back to editable re-verifies from scratch. @@ -129,7 +266,11 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr setVerificationState('checking'); setVerificationMessage(''); testMutation.mutate( - { destination: status?.destination ?? '', publicKey: status?.publicKey ?? '' }, + { + destination: status?.destination ?? '', + publicKey: status?.publicKey ?? '', + expectedTenantId: status?.effectiveTenantId ?? expectedTenantId, + }, { onSuccess: (result) => { if (requestId !== requestRef.current) return; @@ -170,6 +311,8 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr } const configured = status?.configured === true; + const configActive = status?.configActive !== false; + const controlsDisabled = disabled || !configActive; const trimmedPublicKey = publicKey.trim(); const trimmedSecretKey = secretKey.trim(); const destinationChanged = destination !== (status?.destination ?? ''); @@ -182,14 +325,13 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr publicKeyChanged || trimmedSecretKey !== ''; const canSave = - !disabled && + !controlsDisabled && destination !== '' && trimmedPublicKey !== '' && (configured || trimmedSecretKey !== ''); const busy = updateMutation.isPending || testMutation.isPending; const markDraftUnverified = () => { - hasDraftRef.current = true; requestRef.current += 1; setVerificationState('unverified'); setVerificationMessage(''); @@ -214,6 +356,7 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr { destination: nextDestination, publicKey: nextPublicKey, + expectedTenantId, ...(nextSecretKey ? { secretKey: nextSecretKey } : {}), }, { @@ -232,6 +375,108 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr ); }; + /** + * The one place an *explicit* action (save success, conflict rebase) + * adopts a fresh record. Sets `expectedVersion` directly and + * unconditionally instead of leaving it to the passive sync effect above, + * since that effect intentionally freezes the version while a draft + * survives — exactly what an explicit action must NOT do. Guarded by the + * same `isStaleStatus` check as the passive sync effect: an explicit + * action's own read can itself be superseded by a different action that + * already landed a higher version while this one was in flight. + */ + const applyFreshStatus = (fresh: LangfuseConnectionStatus) => { + if (isStaleStatus(fresh)) { + return; + } + const freshTenantId = fresh.effectiveTenantId ?? latestTenantRef.current; + latestTenantRef.current = freshTenantId; + hasAdoptedStatusRef.current = true; + latestVersionRef.current = fresh.configVersion; + setTenantScope(freshTenantId); + setStatus(fresh); + if (!destinationTouchedRef.current) { + setDestination(fresh.destination ?? ''); + } else if (destination === (fresh.destination ?? '')) { + // The surviving draft happens to already match the fresh baseline (e.g. + // a rebase reveals another admin's change that coincides with this + // one) — recompute rather than leave it stuck "touched", or passive + // refreshes would keep freezing expectedVersion for a divergence that + // no longer exists, causing unnecessary 409s. + destinationTouchedRef.current = false; + } + if (!publicKeyTouchedRef.current) { + setPublicKey(fresh.publicKey ?? ''); + } else if (publicKey.trim() === (fresh.publicKey ?? '')) { + publicKeyTouchedRef.current = false; + } + adoptBaseline({ version: fresh.configVersion ?? null, tenantId: freshTenantId, value: fresh }); + }; + + /** Direct reads are version-ordered even when they race outside React Query. */ + const installFreshConnection = (fresh: LangfuseConnectionStatus): LangfuseConnectionStatus => { + return installIfNewer( + queryClient, + (fresh.effectiveTenantId ?? latestTenantRef.current) + ? [...LANGFUSE_CONNECTION_QUERY_KEY, fresh.effectiveTenantId ?? latestTenantRef.current] + : LANGFUSE_CONNECTION_QUERY_KEY, + fresh, + (value) => value.configVersion, + (value) => value.effectiveTenantId, + ); + }; + + const handleUpdateError = (error: Error) => { + if (isVersionConflictError(error)) { + // Do NOT touch the touched refs/destination/publicKey/secretKey here — + // the sync effect above overwrites an untouched destination/publicKey + // from fresh query data, but never touches secretKey or the editing + // flags. Resetting those on conflict left secretKey and the editing + // flags stale against a destination/publicKey the admin never typed. + // Offer an explicit rebase/discard choice instead, same as the generic + // configuration editor. + setVersionConflictOpen(true); + return; + } + notifyError(error.message); + }; + + const handleDiscardAfterConflict = () => + resolveConflict('discard', async () => { + await queryClient.cancelQueries({ queryKey: LANGFUSE_CONNECTION_QUERY_KEY }); + const fetched = await getLangfuseConnectionFn({ data: { expectedTenantId: tenantScope } }); + const fresh = installFreshConnection(fetched); + destinationTouchedRef.current = false; + publicKeyTouchedRef.current = false; + secretKeyDraftRef.current = false; + setSecretKey(''); + setEditingPublicKey(false); + setEditingSecretKey(false); + applyFreshStatus(fresh); + await refreshBaseConfig(queryClient); + }).catch((err: Error) => notifyError(err.message)); + + const handleRebaseAfterConflict = () => + resolveConflict('rebase', async () => { + await queryClient.cancelQueries({ queryKey: LANGFUSE_CONNECTION_QUERY_KEY }); + const fetched = await getLangfuseConnectionFn({ data: { expectedTenantId: tenantScope } }); + const fresh = installFreshConnection(fetched); + if ((fresh.effectiveTenantId ?? latestTenantRef.current) !== expectedTenantId) { + destinationTouchedRef.current = false; + publicKeyTouchedRef.current = false; + secretKeyDraftRef.current = false; + setSecretKey(''); + setEditingPublicKey(false); + setEditingSecretKey(false); + applyFreshStatus(fresh); + await refreshBaseConfig(queryClient); + notifyError(localize('com_config_tenant_changed')); + return; + } + applyFreshStatus(fresh); + await refreshBaseConfig(queryClient); + }).catch((err: Error) => notifyError(err.message)); + const saveConnection = () => { const payload = { // Credential edits are committed through the explicit "Save & enable" action. @@ -239,21 +484,32 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr destination, publicKey: trimmedPublicKey, ...(trimmedSecretKey ? { secretKey: trimmedSecretKey } : {}), + expectedVersion, + expectedTenantId, }; updateMutation.mutate(payload, { - onSuccess: (nextStatus) => { - hasDraftRef.current = false; - queryClient.setQueryData(LANGFUSE_CONNECTION_QUERY_KEY, nextStatus); - testedConnectionRef.current = getConnectionKey(nextStatus); - setStatus(nextStatus); - setDestination(nextStatus.destination ?? ''); - setPublicKey(nextStatus.publicKey ?? ''); + onSuccess: async (nextStatus) => { + destinationTouchedRef.current = false; + publicKeyTouchedRef.current = false; + secretKeyDraftRef.current = false; + await queryClient.cancelQueries({ queryKey: LANGFUSE_CONNECTION_QUERY_KEY }); + const fresh = installFreshConnection(nextStatus); + testedConnectionRef.current = getConnectionKey(fresh); + applyFreshStatus(fresh); setSecretKey(''); setEditingPublicKey(false); setEditingSecretKey(false); notifySuccess(localize('com_config_langfuse_saved')); + // This save itself succeeded regardless of what happens next, so a + // failure here falls back to eventual consistency via invalidation + // instead of surfacing as an error against an action that worked. + try { + await refreshBaseConfig(queryClient); + } catch { + void queryClient.invalidateQueries({ queryKey: baseConfigOptions.queryKey }); + } }, - onError: (error: Error) => notifyError(error.message), + onError: handleUpdateError, }); }; @@ -262,13 +518,16 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr }; const handleCancel = () => { - hasDraftRef.current = false; + destinationTouchedRef.current = false; + publicKeyTouchedRef.current = false; + secretKeyDraftRef.current = false; + const cachedStatus = queryClient.getQueryData(connectionQueryKey); const latestStatus = - queryClient.getQueryData(LANGFUSE_CONNECTION_QUERY_KEY) ?? status; - setStatus(latestStatus); + cachedStatus == null || isStaleStatus(cachedStatus) ? status : cachedStatus; + if (latestStatus) { + applyFreshStatus(latestStatus); + } const storedDestination = latestStatus?.destination; - setDestination(storedDestination ?? ''); - setPublicKey(latestStatus?.publicKey ?? ''); setSecretKey(''); setEditingPublicKey(false); setEditingSecretKey(false); @@ -281,7 +540,7 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr }; const handleEnabledChange = () => { - if (!configured || !status?.destination || !status.publicKey) return; + if (!configActive || !configured || !status?.destination || !status.publicKey) return; const nextEnabled = status.enabled !== true; updateMutation.mutate( @@ -289,15 +548,26 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr enabled: nextEnabled, destination: status.destination, publicKey: status.publicKey, + expectedVersion, + expectedTenantId, }, { - onSuccess: (nextStatus) => { - queryClient.setQueryData(LANGFUSE_CONNECTION_QUERY_KEY, nextStatus); - testedConnectionRef.current = getConnectionKey(nextStatus); - setStatus(nextStatus); + onSuccess: async (nextStatus) => { + await queryClient.cancelQueries({ queryKey: LANGFUSE_CONNECTION_QUERY_KEY }); + const fresh = installFreshConnection(nextStatus); + testedConnectionRef.current = getConnectionKey(fresh); + applyFreshStatus(fresh); notifySuccess(localize('com_config_langfuse_saved')); + // This save itself succeeded regardless of what happens next, so a + // failure here falls back to eventual consistency via invalidation + // instead of surfacing as an error against an action that worked. + try { + await refreshBaseConfig(queryClient); + } catch { + void queryClient.invalidateQueries({ queryKey: baseConfigOptions.queryKey }); + } }, - onError: (error: Error) => notifyError(error.message), + onError: handleUpdateError, }, ); }; @@ -327,13 +597,22 @@ export function LangfuseRenderer({ disabled, isEditingScope }: t.FieldRendererPr {statusLabel}
+ {!configActive && ( +

+ {localize('com_config_langfuse_config_inactive')} +

+ )} +