improvement(access-control): edit group details, filter by status, and colocate chat deploy auth (#5902)

* improvement(access-control): editable group details, status filters, block tooltips, chat auth colocation

- General tab gains editable Name and Description fields wired into the
  existing dirty buffer, so the header Save/Discard chips and the
  unsaved-changes guard cover them. Save only sends changed fields and
  surfaces the route's duplicate-name 409.
- Blocks, Model Providers, and Platform tabs gain an All/Enabled/Disabled
  filter beside their search field, evaluated against the editing buffer.
- Every block row carries an Info badge with the block's description. The
  badge sits outside the label/expand button so it never toggles the row.
- The chat deploy toggle moves out of Deploy Tabs into the Chat section
  alongside its allowed-auth-modes dropdown, mirroring the Files section.

* improvement(access-control): polish group detail filters, tooltips, and details fields

Follows the cleanup review:
- reconcile the post-save baseline from the server response instead of
  local values, matching the scope/default writes
- pin the status-filter dropdown width so the search field stops resizing
- restore flex-1 on the block-name button and move the row hover surface
  to the wrapper so Info badges align in a column
- add empty states for filter-empty lists and neutralize Select All there
- surface a 'Name is required' message next to the disabled Save
- align hint text on the field-hint tokens and drop a redundant TSDoc

* refactor(access-control): keep chat and files toggles in the platform registry

The first pass pulled hideDeployChatbot out of the declarative platformFeatures
array and hand-rolled a Chat section beside the existing bespoke Files one. That
forfeited search, status filtering, Select All, category grouping and the Info
hint, and the replacement platformSectionVisible re-implemented two of those
with different semantics — searching 'deploy' or 'deployment' hid the very
control named Deployment, and Select All silently skipped both toggles.

Both toggles are now ordinary registry entries under their own Chat and Files
categories, with an id-keyed featureExtras map supplying the nested auth-mode
dropdown. Search, filtering, Select All, hints and the empty state are correct
by construction, and the parallel filter pipeline is gone.

Also from the review:
- index the allow-lists into Sets so per-row membership checks are O(1)
- split the search and status passes so the common 'all' filter returns the
  searched list by reference and a checkbox toggle no longer re-sorts ~180 rows
- extract StatusFilterChip and AuthModeField instead of stamping out the
  dropdowns three and two times
- derive nameChanged/descriptionChanged once instead of repeating the
  comparisons in the save payload
- lock the config key-order invariant the dirty check depends on with a test

* polish(access-control): apply the second cleanup round

- indent the nested auth-mode field so it lines up under its toggle's label
  instead of reading as a sibling row, and label the dropdown for screen readers
- order Chat right after Deploy Tabs so the three deploy targets stay adjacent
- flush the trailing Select All chips on the providers and platform rows
- drop the doubled margin on the name error (SettingRow already gaps it)
- hoist PLATFORM_FEATURES and PLATFORM_CATEGORY_ORDER to module scope
- drop the useCallback on the two save/discard handlers; nothing observes their
  identity and their deps changed on every keystroke anyway
- fix a comment that still pointed at a 'Hide Chat' toggle that no longer exists

* fix(access-control): keep the block disclosure chevron inside its toggle button

Splitting the chevron out so the Info badge could sit beside the name left the
chevron with no click handler — the visible expansion affordance did nothing.
It goes back inside the button; Info stays outside it, since an Info trigger is
itself a button and cannot nest.

* chore(access-control): use structuredClone in the config key-order test

check:utils bans JSON.parse(JSON.stringify(...)). The clone only needs to hand
the schema a distinct object; structuredClone preserves key order the same way.

* fix(access-control): trim descriptions so a padded value can't wedge the form

descriptionChanged compared a trimmed draft against the raw saved description,
so a group whose stored description carried padding opened dirty and could
never be cleared — Discard restored the same padded string, and the
unsaved-changes guard then blocked navigation until a save rewrote it.

The contract now trims description on create and update, matching what name
already did, and the dirty check trims both sides so existing padded rows
behave too.

* improvement(access-control): seed the description buffer trimmed

Keeps the editing buffer and the dirty baseline normalized the same way, so a
legacy row with padding no longer shows stray whitespace in the input and the
buffer never round-trips padding a save would strip anyway.

* fix(emcn): forward aria-label/aria-labelledby from ChipDropdown to its trigger

ChipDropdown destructures only its known props, so an aria-labelledby passed by
a consumer never reached the trigger button — AuthModeField's wiring to its
visible label was silently dropped and the control had no accessible name.

Both attributes are now explicit, typed props forwarded to the trigger. Kept as
two named props rather than a rest spread so the component still owns its
chrome and consumers can't smuggle arbitrary attributes onto the button.

* fix(access-control): correct the nested field indent, platform row hover, and aria name

- aria-labelledby REPLACES the content-derived name, so naming the auth-mode
  dropdown with its caption alone dropped the selected value from the accessible
  name — worse than no attribute. It now references caption + trigger, which
  needed ChipDropdown to forward id as well.
- The nested field's pl-[30px] assumed a 14px checkbox; the default is 16px, so
  the caption sat 2px left of the label it hangs under while the dropdown (not
  flush, so mx-0.5) sat at 32. Both are pl-8 + flush now.
- Platform feature rows kept their hover surface on the label, so the highlight
  stopped 20px short of the row edge while the Blocks tab ran flush. Moved to
  the wrapper, matching the core-blocks cell.
- Split the platform search and status passes like the provider and block lists,
  so a toggle no longer recomputes three chained memos.
- Dropped an unreachable allLabel and a comment that would go stale on merge.

* fix(access-control): trim the name comparison the same way as description

nameChanged compared a trimmed buffer against an untrimmed baseline — the exact
asymmetry already fixed for description. A group stored before the name schema
gained .trim() opens permanently dirty: Save/Discard visible and the
unsaved-changes modal firing on back, until a save rewrites it.

Both buffers now seed trimmed and compare trimmed on both sides. Also dims the
Info badge along with the row it belongs to when the block is disallowed.
This commit is contained in:
Waleed
2026-07-23 15:44:13 -07:00
committed by GitHub
parent 6f90d8976d
commit 3796e9db4f
4 changed files with 564 additions and 291 deletions
@@ -1,6 +1,6 @@
'use client'
import { useCallback, useMemo, useRef, useState } from 'react'
import { type ReactNode, useCallback, useId, useMemo, useRef, useState } from 'react'
import {
Checkbox,
Chip,
@@ -37,6 +37,7 @@ import {
} from '@/app/workspace/[workspaceId]/settings/components/member-list'
import { RowActionsMenu } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu'
import { saveDiscardActions } from '@/app/workspace/[workspaceId]/settings/components/save-discard-actions/save-discard-actions'
import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state'
import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel'
import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section'
import { useSettingsUnsavedGuard } from '@/app/workspace/[workspaceId]/settings/hooks/use-settings-unsaved-guard'
@@ -53,6 +54,7 @@ import {
useRemovePermissionGroupMember,
useUpdatePermissionGroup,
} from '@/ee/access-control/hooks/permission-groups'
import { SettingRow } from '@/ee/components/setting-row'
import { useBlacklistedProviders } from '@/hooks/queries/allowed-providers'
import { useOrganizationRoster } from '@/hooks/queries/organization'
import { useProviderModels } from '@/hooks/queries/providers'
@@ -90,6 +92,227 @@ const ALL_CHAT_DEPLOY_AUTH_TYPES: ShareAuthType[] = CHAT_DEPLOY_AUTH_TYPE_OPTION
(o) => o.value
)
type StatusFilter = 'all' | 'enabled' | 'disabled'
const STATUS_FILTER_OPTIONS: { value: StatusFilter; label: string }[] = [
{ value: 'all', label: 'Show all' },
{ value: 'enabled', label: 'Show enabled' },
{ value: 'disabled', label: 'Show disabled' },
]
function matchesStatusFilter(filter: StatusFilter, enabled: boolean) {
return filter === 'all' || (filter === 'enabled') === enabled
}
interface StatusFilterChipProps {
value: StatusFilter
onChange: (value: StatusFilter) => void
/** Set when the chip is the last control in its row, so it sits flush to the edge. */
flush?: boolean
}
/** The All/Enabled/Disabled narrowing control shared by the three list tabs. */
function StatusFilterChip({ value, onChange, flush }: StatusFilterChipProps) {
return (
<ChipDropdown
value={value}
onChange={(next) => onChange(next as StatusFilter)}
options={STATUS_FILTER_OPTIONS}
matchTriggerWidth={false}
flush={flush}
className='w-[140px] flex-shrink-0'
/>
)
}
interface AuthModeFieldProps {
label: string
value: ShareAuthType[]
onChange: (values: string[]) => void
options: { value: ShareAuthType; label: string }[]
disabled: boolean
}
/**
* The allowed-auth-modes multi-select nested under a platform toggle. Dims and
* disables together with the toggle that owns it. The left padding lines both
* children up with the parent's label text — row gutter (8) + checkbox (16) +
* gap (8) = 32 — so the field reads as subordinate rather than as a sibling row.
* The dropdown is `flush` so its own `mx-0.5` doesn't push it 2px past the
* caption above it.
*/
function AuthModeField({ label, value, onChange, options, disabled }: AuthModeFieldProps) {
const labelId = useId()
const triggerId = useId()
return (
<div className={cn('flex flex-col gap-1.5 pt-1 pr-2 pb-2 pl-8', disabled && 'opacity-50')}>
<span id={labelId} className='text-[var(--text-muted)] text-caption'>
{label}
</span>
<ChipDropdown
multiple
flush
showAllOption={false}
id={triggerId}
// Both ids: `aria-labelledby` replaces the content-derived name, so
// naming it with the label alone would drop the selected value.
aria-labelledby={`${labelId} ${triggerId}`}
value={value}
onChange={onChange}
options={options}
disabled={disabled}
matchTriggerWidth={false}
className='w-[200px]'
/>
</div>
)
}
/** Render order for the platform-feature category sections; unlisted ones follow. */
const PLATFORM_CATEGORY_ORDER = [
'Sidebar',
'Deploy Tabs',
'Chat',
'Collaboration',
'Workflow Panel',
'Tools',
'Features',
'Settings Tabs',
'Logs',
'Files',
]
const PLATFORM_FEATURES = [
{
id: 'hide-knowledge-base',
label: 'Knowledge Base',
category: 'Sidebar',
configKey: 'hideKnowledgeBaseTab' as const,
hint: 'Hide the Knowledge Base module from the sidebar.',
},
{
id: 'hide-tables',
label: 'Tables',
category: 'Sidebar',
configKey: 'hideTablesTab' as const,
hint: 'Hide the Tables module from the sidebar.',
},
{
id: 'hide-copilot',
label: 'Chat',
category: 'Workflow Panel',
configKey: 'hideCopilot' as const,
hint: 'Hide the Chat panel so users cannot build or edit with natural language.',
},
{
id: 'hide-integrations',
label: 'Integrations',
category: 'Settings Tabs',
configKey: 'hideIntegrationsTab' as const,
hint: 'Hide the Integrations settings tab (OAuth connections).',
},
{
id: 'hide-secrets',
label: 'Secrets',
category: 'Settings Tabs',
configKey: 'hideSecretsTab' as const,
hint: 'Hide the Secrets (environment variables) settings tab.',
},
{
id: 'hide-api-keys',
label: 'API Keys',
category: 'Settings Tabs',
configKey: 'hideApiKeysTab' as const,
hint: 'Hide the API Keys settings tab.',
},
{
id: 'hide-files',
label: 'Files',
category: 'Settings Tabs',
configKey: 'hideFilesTab' as const,
hint: 'Hide the Files settings tab.',
},
{
id: 'hide-deploy-api',
label: 'API',
category: 'Deploy Tabs',
configKey: 'hideDeployApi' as const,
hint: 'Hide the API deployment option.',
},
{
id: 'hide-deploy-mcp',
label: 'MCP',
category: 'Deploy Tabs',
configKey: 'hideDeployMcp' as const,
hint: 'Hide the MCP server deployment option.',
},
{
id: 'disable-mcp',
label: 'MCP Tools',
category: 'Tools',
configKey: 'disableMcpTools' as const,
hint: 'Block agents from calling MCP tools.',
},
{
id: 'disable-custom-tools',
label: 'Custom Tools',
category: 'Tools',
configKey: 'disableCustomTools' as const,
hint: 'Block agents from calling user-defined custom tools.',
},
{
id: 'disable-skills',
label: 'Skills',
category: 'Tools',
configKey: 'disableSkills' as const,
hint: 'Block agents from loading skills.',
},
{
id: 'hide-trace-spans',
label: 'Trace Spans',
category: 'Logs',
configKey: 'hideTraceSpans' as const,
hint: 'Hide per-block trace spans in logs.',
},
{
id: 'disable-invitations',
label: 'Invitations',
category: 'Collaboration',
configKey: 'disableInvitations' as const,
hint: 'Prevent users from inviting others to workspaces.',
},
{
id: 'hide-inbox',
label: 'Sim Mailer',
category: 'Features',
configKey: 'hideInboxTab' as const,
hint: 'Hide the Sim Mailer inbox.',
},
{
id: 'disable-public-api',
label: 'Public API',
category: 'Features',
configKey: 'disablePublicApi' as const,
hint: 'Disable public API access to deployed workflows.',
},
// Chat and Files get a category of their own so their nested auth-mode
// dropdown (see `featureExtras`) reads as part of the toggle it qualifies.
{
id: 'hide-deploy-chatbot',
label: 'Deployment',
category: 'Chat',
configKey: 'hideDeployChatbot' as const,
hint: 'Hide the chat deployment option.',
},
{
id: 'disable-public-file-sharing',
label: 'Public Sharing',
category: 'Files',
configKey: 'disablePublicFileSharing' as const,
hint: 'Disable public file-share links.',
},
]
interface OrganizationMemberOption {
userId: string
user: {
@@ -521,7 +744,7 @@ function BlockToolRow({
onClick={() => isBlockAllowed && isExpandable && setExpanded((prev) => !prev)}
disabled={!isBlockAllowed || !isExpandable}
className={cn(
'flex flex-1 items-center gap-2 text-left',
'flex min-w-0 flex-1 items-center gap-2 text-left',
isBlockAllowed && isExpandable ? 'cursor-pointer' : 'cursor-default',
!isBlockAllowed && 'opacity-60'
)}
@@ -541,6 +764,12 @@ function BlockToolRow({
/>
)}
</button>
{/* Outside the button: an Info trigger is itself a button and cannot nest. */}
{block.description && (
<Info side='top' className={cn('flex-shrink-0', !isBlockAllowed && 'opacity-60')}>
{block.description}
</Info>
)}
</div>
{expanded && isBlockAllowed && isExpandable && (
<div className='border-[var(--border)] border-t px-2 pt-2 pb-3'>
@@ -595,11 +824,15 @@ export function GroupDetail({
*/
const [viewingGroup, setViewingGroup] = useState<PermissionGroup>(group)
const [editingConfig, setEditingConfig] = useState<PermissionGroupConfig>({ ...group.config })
const [editingName, setEditingName] = useState(group.name.trim())
const [editingDescription, setEditingDescription] = useState((group.description ?? '').trim())
const prevGroupIdRef = useRef(group.id)
if (prevGroupIdRef.current !== group.id) {
prevGroupIdRef.current = group.id
setViewingGroup(group)
setEditingConfig({ ...group.config })
setEditingName(group.name.trim())
setEditingDescription((group.description ?? '').trim())
}
/**
@@ -613,6 +846,9 @@ export function GroupDetail({
const [providerSearchTerm, setProviderSearchTerm] = useState('')
const [integrationSearchTerm, setIntegrationSearchTerm] = useState('')
const [platformSearchTerm, setPlatformSearchTerm] = useState('')
const [providerStatusFilter, setProviderStatusFilter] = useState<StatusFilter>('all')
const [blockStatusFilter, setBlockStatusFilter] = useState<StatusFilter>('all')
const [platformStatusFilter, setPlatformStatusFilter] = useState<StatusFilter>('all')
const [showAddMembersModal, setShowAddMembersModal] = useState(false)
const [addMembersError, setAddMembersError] = useState<string | null>(null)
@@ -676,141 +912,24 @@ export function GroupDetail({
return map
}, [allBlocks])
const platformFeatures = useMemo(
() => [
{
id: 'hide-knowledge-base',
label: 'Knowledge Base',
category: 'Sidebar',
configKey: 'hideKnowledgeBaseTab' as const,
hint: 'Hide the Knowledge Base module from the sidebar.',
},
{
id: 'hide-tables',
label: 'Tables',
category: 'Sidebar',
configKey: 'hideTablesTab' as const,
hint: 'Hide the Tables module from the sidebar.',
},
{
id: 'hide-copilot',
label: 'Chat',
category: 'Workflow Panel',
configKey: 'hideCopilot' as const,
hint: 'Hide the Chat panel so users cannot build or edit with natural language.',
},
{
id: 'hide-integrations',
label: 'Integrations',
category: 'Settings Tabs',
configKey: 'hideIntegrationsTab' as const,
hint: 'Hide the Integrations settings tab (OAuth connections).',
},
{
id: 'hide-secrets',
label: 'Secrets',
category: 'Settings Tabs',
configKey: 'hideSecretsTab' as const,
hint: 'Hide the Secrets (environment variables) settings tab.',
},
{
id: 'hide-api-keys',
label: 'API Keys',
category: 'Settings Tabs',
configKey: 'hideApiKeysTab' as const,
hint: 'Hide the API Keys settings tab.',
},
{
id: 'hide-files',
label: 'Files',
category: 'Settings Tabs',
configKey: 'hideFilesTab' as const,
hint: 'Hide the Files settings tab.',
},
{
id: 'hide-deploy-api',
label: 'API',
category: 'Deploy Tabs',
configKey: 'hideDeployApi' as const,
hint: 'Hide the API deployment option.',
},
{
id: 'hide-deploy-mcp',
label: 'MCP',
category: 'Deploy Tabs',
configKey: 'hideDeployMcp' as const,
hint: 'Hide the MCP server deployment option.',
},
{
id: 'hide-deploy-chatbot',
label: 'Chat',
category: 'Deploy Tabs',
configKey: 'hideDeployChatbot' as const,
hint: 'Hide the chatbot deployment option.',
},
{
id: 'disable-mcp',
label: 'MCP Tools',
category: 'Tools',
configKey: 'disableMcpTools' as const,
hint: 'Block agents from calling MCP tools.',
},
{
id: 'disable-custom-tools',
label: 'Custom Tools',
category: 'Tools',
configKey: 'disableCustomTools' as const,
hint: 'Block agents from calling user-defined custom tools.',
},
{
id: 'disable-skills',
label: 'Skills',
category: 'Tools',
configKey: 'disableSkills' as const,
hint: 'Block agents from loading skills.',
},
{
id: 'hide-trace-spans',
label: 'Trace Spans',
category: 'Logs',
configKey: 'hideTraceSpans' as const,
hint: 'Hide per-block trace spans in logs.',
},
{
id: 'disable-invitations',
label: 'Invitations',
category: 'Collaboration',
configKey: 'disableInvitations' as const,
hint: 'Prevent users from inviting others to workspaces.',
},
{
id: 'hide-inbox',
label: 'Sim Mailer',
category: 'Features',
configKey: 'hideInboxTab' as const,
hint: 'Hide the Sim Mailer inbox.',
},
{
id: 'disable-public-api',
label: 'Public API',
category: 'Features',
configKey: 'disablePublicApi' as const,
hint: 'Disable public API access to deployed workflows.',
},
],
[]
)
const filteredPlatformFeatures = useMemo(() => {
if (!platformSearchTerm.trim()) return platformFeatures
const search = platformSearchTerm.toLowerCase()
return platformFeatures.filter(
const searchedPlatformFeatures = useMemo(() => {
const search = platformSearchTerm.trim().toLowerCase()
if (!search) return PLATFORM_FEATURES
return PLATFORM_FEATURES.filter(
(f) => f.label.toLowerCase().includes(search) || f.category.toLowerCase().includes(search)
)
}, [platformFeatures, platformSearchTerm])
}, [platformSearchTerm])
/** Split from the search pass for the same reason as the provider and block lists. */
const filteredPlatformFeatures = useMemo(() => {
if (platformStatusFilter === 'all') return searchedPlatformFeatures
return searchedPlatformFeatures.filter((f) =>
matchesStatusFilter(platformStatusFilter, !editingConfig[f.configKey])
)
}, [searchedPlatformFeatures, platformStatusFilter, editingConfig])
const platformCategories = useMemo(() => {
const categories: Record<string, typeof platformFeatures> = {}
const categories: Record<string, typeof PLATFORM_FEATURES> = {}
for (const feature of filteredPlatformFeatures) {
if (!categories[feature.category]) {
categories[feature.category] = []
@@ -821,19 +940,9 @@ export function GroupDetail({
}, [filteredPlatformFeatures])
const platformCategorySections = useMemo(() => {
const order = [
'Sidebar',
'Deploy Tabs',
'Collaboration',
'Workflow Panel',
'Tools',
'Features',
'Settings Tabs',
'Logs',
]
const known = order.filter((c) => platformCategories[c]?.length)
const known = PLATFORM_CATEGORY_ORDER.filter((c) => platformCategories[c]?.length)
const extras = Object.keys(platformCategories).filter(
(c) => c !== 'Files' && !order.includes(c) && platformCategories[c]?.length
(c) => !PLATFORM_CATEGORY_ORDER.includes(c) && platformCategories[c]?.length
)
return [...known, ...extras].map((category) => ({
category,
@@ -844,20 +953,82 @@ export function GroupDetail({
const hasConfigChanges = useMemo(() => {
return JSON.stringify(viewingGroup.config) !== JSON.stringify(editingConfig)
}, [viewingGroup.config, editingConfig])
const guard = useSettingsUnsavedGuard({ isDirty: hasConfigChanges })
const filteredProviders = useMemo(() => {
if (!providerSearchTerm.trim()) return allProviderIds
const query = providerSearchTerm.toLowerCase()
// Both buffers are seeded trimmed and compared against a trimmed baseline. The
// contract trims name and description on write, but a row stored before those
// schemas gained `.trim()` (or written straight to the API) can still carry
// padding — compared raw it would open dirty with no way to clear it, since
// Discard restores the same padded value.
const trimmedName = editingName.trim()
const trimmedDescription = editingDescription.trim()
const nameChanged = trimmedName !== viewingGroup.name.trim()
const descriptionChanged = trimmedDescription !== (viewingGroup.description ?? '').trim()
const hasChanges = hasConfigChanges || nameChanged || descriptionChanged
const guard = useSettingsUnsavedGuard({ isDirty: hasChanges })
/**
* `null` means "everything allowed". Indexing the allow-lists once keeps the
* per-row membership checks O(1) — they run for every one of the ~200 block
* rows on each render, and again in the section-wide `every(...)` scans.
*/
const allowedIntegrationSet = useMemo(
() =>
editingConfig.allowedIntegrations === null
? null
: new Set(editingConfig.allowedIntegrations),
[editingConfig.allowedIntegrations]
)
const allowedProviderSet = useMemo(
() =>
editingConfig.allowedModelProviders === null
? null
: new Set(editingConfig.allowedModelProviders),
[editingConfig.allowedModelProviders]
)
const isIntegrationAllowed = useCallback(
(blockType: string) => allowedIntegrationSet === null || allowedIntegrationSet.has(blockType),
[allowedIntegrationSet]
)
const isProviderAllowed = useCallback(
(providerId: string) => allowedProviderSet === null || allowedProviderSet.has(providerId),
[allowedProviderSet]
)
const searchedProviders = useMemo(() => {
const query = providerSearchTerm.trim().toLowerCase()
if (!query) return allProviderIds
return allProviderIds.filter((id) => id.toLowerCase().includes(query))
}, [allProviderIds, providerSearchTerm])
const filteredBlocks = useMemo(() => {
if (!integrationSearchTerm.trim()) return visibleBlocks
const query = integrationSearchTerm.toLowerCase()
/**
* Split from the search pass so the common `all` case returns the searched
* list by reference — only the status pass depends on the allow-list, so a
* checkbox toggle no longer invalidates downstream consumers.
*/
const filteredProviders = useMemo(() => {
if (providerStatusFilter === 'all') return searchedProviders
return searchedProviders.filter((id) =>
matchesStatusFilter(providerStatusFilter, isProviderAllowed(id))
)
}, [searchedProviders, providerStatusFilter, isProviderAllowed])
const searchedBlocks = useMemo(() => {
const query = integrationSearchTerm.trim().toLowerCase()
if (!query) return visibleBlocks
return visibleBlocks.filter((b) => b.name.toLowerCase().includes(query))
}, [visibleBlocks, integrationSearchTerm])
const filteredBlocks = useMemo(() => {
if (blockStatusFilter === 'all') return searchedBlocks
return searchedBlocks.filter((b) =>
matchesStatusFilter(blockStatusFilter, isIntegrationAllowed(b.type))
)
}, [searchedBlocks, blockStatusFilter, isIntegrationAllowed])
const filteredCoreBlocks = useMemo(
() => filteredBlocks.filter((block) => block.category === 'blocks'),
[filteredBlocks]
@@ -886,13 +1057,6 @@ export function GroupDetail({
return organizationMembers.filter((m) => !existingMemberUserIds.has(m.userId))
}, [organizationMembers, members])
const isIntegrationAllowed = useCallback(
(blockType: string) =>
editingConfig.allowedIntegrations === null ||
editingConfig.allowedIntegrations.includes(blockType),
[editingConfig.allowedIntegrations]
)
/**
* Drops denied tools whose integration is no longer allowed, keeping the
* invariant that `deniedTools` only holds tools of currently-allowed blocks.
@@ -1003,13 +1167,6 @@ export function GroupDetail({
return counts
}, [editingConfig.deniedTools, allBlocks])
const isProviderAllowed = useCallback(
(providerId: string) =>
editingConfig.allowedModelProviders === null ||
editingConfig.allowedModelProviders.includes(providerId),
[editingConfig.allowedModelProviders]
)
const toggleProvider = useCallback(
(providerId: string) => {
setEditingConfig((prev) => {
@@ -1130,7 +1287,7 @@ export function GroupDetail({
const setChatDeployAuthTypes = useCallback((values: string[]) => {
// At least one mode must stay allowed while chat deploy is enabled — an empty
// allow-list would silently block every chat deployment. To turn chat deploy
// off entirely, use the Hide Chat toggle instead.
// off entirely, uncheck Chat → Deployment instead.
if (values.length === 0) return
setEditingConfig((prev) => ({
...prev,
@@ -1139,26 +1296,66 @@ export function GroupDetail({
}))
}, [])
/** Persists the editing buffer. */
const handleSaveConfig = useCallback(async () => {
/**
* Nested controls rendered under a platform feature's checkbox, keyed by
* feature id. Kept out of `PLATFORM_FEATURES` so that array stays pure data.
*/
const featureExtras: Partial<Record<string, ReactNode>> = {
'hide-deploy-chatbot': (
<AuthModeField
label='Auth modes chat deployments may use'
value={chatDeployAuthValue}
onChange={setChatDeployAuthTypes}
options={CHAT_DEPLOY_AUTH_TYPE_OPTIONS}
disabled={editingConfig.hideDeployChatbot}
/>
),
'disable-public-file-sharing': (
<AuthModeField
label='Auth modes public file-share links may use'
value={fileShareAuthValue}
onChange={setFileShareAuthTypes}
options={FILE_SHARE_AUTH_TYPE_OPTIONS}
disabled={editingConfig.disablePublicFileSharing}
/>
),
}
/** Persists the editing buffer — name/description are only sent when they changed. */
const handleSaveConfig = async () => {
if (!trimmedName) return
try {
await updatePermissionGroup.mutateAsync({
const result = await updatePermissionGroup.mutateAsync({
id: viewingGroup.id,
organizationId,
config: editingConfig,
...(hasConfigChanges && { config: editingConfig }),
...(nameChanged && { name: trimmedName }),
...(descriptionChanged && { description: trimmedDescription || null }),
})
setViewingGroup((prev) => ({ ...prev, config: editingConfig }))
// Reconcile from the server's copy, like the scope/default writes do, so a
// server-side normalization can't leave the dirty check comparing against a
// baseline that was never persisted. Editing buffers are left alone so
// in-flight edits survive and correctly re-mark the form dirty.
const saved = result.permissionGroup
setViewingGroup((prev) => ({
...prev,
config: saved.config,
name: saved.name,
description: saved.description,
}))
} catch (error) {
logger.error('Failed to update config', error)
logger.error('Failed to save permission group', error)
toast.error("Couldn't save changes", {
description: getErrorMessage(error, 'Please try again in a moment.'),
})
}
}, [viewingGroup.id, editingConfig, organizationId, updatePermissionGroup])
}
const handleDiscardConfig = useCallback(() => {
const handleDiscardConfig = () => {
setEditingConfig({ ...viewingGroup.config })
}, [viewingGroup.config])
setEditingName(viewingGroup.name.trim())
setEditingDescription((viewingGroup.description ?? '').trim())
}
const handleBack = useCallback(() => {
guard.guardBack(onBack)
@@ -1307,10 +1504,11 @@ export function GroupDetail({
description={viewingGroup.description ?? undefined}
actions={[
...saveDiscardActions({
dirty: hasConfigChanges,
dirty: hasChanges,
saving: updatePermissionGroup.isPending,
onSave: handleSaveConfig,
onDiscard: handleDiscardConfig,
saveDisabled: !trimmedName,
}),
{
text: deletePermissionGroup.isPending ? 'Deleting...' : 'Delete',
@@ -1330,6 +1528,31 @@ export function GroupDetail({
{configTab === 'general' && (
<>
<SettingsSection label='Details'>
<div className='flex flex-col gap-4'>
<SettingRow label='Name'>
<ChipInput
value={editingName}
onChange={(e) => setEditingName(e.target.value)}
placeholder='e.g., Marketing Team'
maxLength={100}
error={!trimmedName}
/>
{!trimmedName && (
<p className='text-[var(--text-error)] text-caption'>Name is required.</p>
)}
</SettingRow>
<SettingRow label='Description'>
<ChipInput
value={editingDescription}
onChange={(e) => setEditingDescription(e.target.value)}
placeholder='e.g., Limited access for marketing users'
maxLength={500}
/>
</SettingRow>
</div>
</SettingsSection>
<SettingsSection label='Default group'>
<div className='flex items-center justify-between gap-3'>
<span className='text-[var(--text-muted)] text-small'>
@@ -1457,27 +1680,36 @@ export function GroupDetail({
onChange={(e) => setProviderSearchTerm(e.target.value)}
className='min-w-0 flex-1'
/>
<StatusFilterChip value={providerStatusFilter} onChange={setProviderStatusFilter} />
<Chip
flush
onClick={() => setProvidersAllowed(filteredProviders, !filteredProvidersAllAllowed)}
disabled={filteredProviders.length === 0}
>
{filteredProvidersAllAllowed ? 'Deselect All' : 'Select All'}
</Chip>
</div>
<div className='flex flex-col gap-0.5'>
{filteredProviders.map((providerId) => (
<ProviderRow
key={providerId}
providerId={providerId}
isProviderAllowed={isProviderAllowed(providerId)}
onToggleProvider={() => toggleProvider(providerId)}
deniedCount={deniedCountByProvider[providerId] ?? 0}
workspaceId={workspaceId}
isAllowed={isModelAllowed}
onToggle={toggleModel}
onSetDenied={setModelsDenied}
/>
))}
</div>
{filteredProviders.length === 0 ? (
<SettingsEmptyState variant='inline'>
No providers match your filters.
</SettingsEmptyState>
) : (
<div className='flex flex-col gap-0.5'>
{filteredProviders.map((providerId) => (
<ProviderRow
key={providerId}
providerId={providerId}
isProviderAllowed={isProviderAllowed(providerId)}
onToggleProvider={() => toggleProvider(providerId)}
deniedCount={deniedCountByProvider[providerId] ?? 0}
workspaceId={workspaceId}
isAllowed={isModelAllowed}
onToggle={toggleModel}
onSetDenied={setModelsDenied}
/>
))}
</div>
)}
</div>
)}
@@ -1491,7 +1723,13 @@ export function GroupDetail({
onChange={(e) => setIntegrationSearchTerm(e.target.value)}
className='min-w-0 flex-1'
/>
<StatusFilterChip value={blockStatusFilter} onChange={setBlockStatusFilter} flush />
</div>
{filteredCoreBlocks.length === 0 && filteredToolBlocks.length === 0 && (
<SettingsEmptyState variant='inline'>
No blocks match your filters.
</SettingsEmptyState>
)}
{filteredCoreBlocks.length > 0 && (
<SettingsSection
label='Core Blocks'
@@ -1509,24 +1747,33 @@ export function GroupDetail({
const BlockIcon = block.icon
const checkboxId = `block-${block.type}`
return (
<label
<div
key={block.type}
htmlFor={checkboxId}
className='flex cursor-pointer items-center gap-2 rounded-md px-2 py-[5px] transition-colors hover-hover:bg-[var(--surface-active)]'
className='flex items-center gap-1.5 rounded-md pr-2 transition-colors hover-hover:bg-[var(--surface-active)]'
>
<Checkbox
id={checkboxId}
checked={isIntegrationAllowed(block.type)}
onCheckedChange={() => toggleIntegration(block.type)}
/>
<div
className='relative flex size-[16px] flex-shrink-0 items-center justify-center overflow-hidden rounded-sm'
style={{ background: block.bgColor }}
<label
htmlFor={checkboxId}
className='flex min-w-0 flex-1 cursor-pointer items-center gap-2 py-[5px] pl-2'
>
{BlockIcon && <BlockIcon className='!size-[9px] text-white' />}
</div>
<span className='truncate font-medium text-sm'>{block.name}</span>
</label>
<Checkbox
id={checkboxId}
checked={isIntegrationAllowed(block.type)}
onCheckedChange={() => toggleIntegration(block.type)}
/>
<div
className='relative flex size-[16px] flex-shrink-0 items-center justify-center overflow-hidden rounded-sm'
style={{ background: block.bgColor }}
>
{BlockIcon && <BlockIcon className='!size-[9px] text-white' />}
</div>
<span className='truncate font-medium text-sm'>{block.name}</span>
</label>
{block.description && (
<Info side='top' className='flex-shrink-0'>
{block.description}
</Info>
)}
</div>
)
})}
</div>
@@ -1579,6 +1826,7 @@ export function GroupDetail({
onChange={(e) => setPlatformSearchTerm(e.target.value)}
className='min-w-0 flex-1'
/>
<StatusFilterChip value={platformStatusFilter} onChange={setPlatformStatusFilter} />
<Chip
onClick={() =>
setEditingConfig((prev) => ({
@@ -1588,101 +1836,49 @@ export function GroupDetail({
),
}))
}
flush
disabled={filteredPlatformFeatures.length === 0}
>
{platformAllVisible ? 'Deselect All' : 'Select All'}
</Chip>
</div>
{platformCategorySections.length === 0 && (
<SettingsEmptyState variant='inline'>
No features match your filters.
</SettingsEmptyState>
)}
{platformCategorySections.map(({ category, features }) => (
<SettingsSection key={category} label={category}>
<div className='flex flex-col gap-0.5'>
{features.map((feature) => (
<div key={feature.id} className='flex items-center gap-1.5'>
<label
htmlFor={feature.id}
className='flex flex-1 cursor-pointer items-center gap-2 rounded-md px-2 py-[5px] transition-colors hover-hover:bg-[var(--surface-active)]'
>
<Checkbox
id={feature.id}
checked={!editingConfig[feature.configKey]}
onCheckedChange={(checked) =>
setEditingConfig((prev) => ({
...prev,
[feature.configKey]: checked !== true,
}))
}
/>
<span className='font-normal text-sm'>{feature.label}</span>
</label>
<Info side='top'>{feature.hint}</Info>
<div key={feature.id} className='flex flex-col'>
<div className='flex items-center gap-1.5 rounded-md pr-2 transition-colors hover-hover:bg-[var(--surface-active)]'>
<label
htmlFor={feature.id}
className='flex flex-1 cursor-pointer items-center gap-2 py-[5px] pl-2'
>
<Checkbox
id={feature.id}
checked={!editingConfig[feature.configKey]}
onCheckedChange={(checked) =>
setEditingConfig((prev) => ({
...prev,
[feature.configKey]: checked !== true,
}))
}
/>
<span className='font-normal text-sm'>{feature.label}</span>
</label>
<Info side='top' className='flex-shrink-0'>
{feature.hint}
</Info>
</div>
{featureExtras[feature.id]}
</div>
))}
</div>
</SettingsSection>
))}
<SettingsSection label='Chat'>
<div
className={cn(
'flex flex-col gap-1.5 px-2 pt-1',
editingConfig.hideDeployChatbot && 'opacity-50'
)}
>
<span className='text-[var(--text-secondary)] text-xs'>
Auth modes chat deployments may use
</span>
<ChipDropdown
multiple
showAllOption={false}
allLabel='None'
value={chatDeployAuthValue}
onChange={setChatDeployAuthTypes}
options={CHAT_DEPLOY_AUTH_TYPE_OPTIONS}
disabled={editingConfig.hideDeployChatbot}
matchTriggerWidth={false}
className='w-[200px]'
/>
</div>
</SettingsSection>
<SettingsSection label='Files'>
<div className='flex flex-col gap-1.5'>
<label
htmlFor='disable-public-file-sharing'
className='flex cursor-pointer items-center gap-2 rounded-md px-2 py-[5px] transition-colors hover-hover:bg-[var(--surface-active)]'
>
<Checkbox
id='disable-public-file-sharing'
checked={!editingConfig.disablePublicFileSharing}
onCheckedChange={(checked) =>
setEditingConfig((prev) => ({
...prev,
disablePublicFileSharing: checked !== true,
}))
}
/>
<span className='font-normal text-sm'>Public Sharing</span>
</label>
<div
className={cn(
'flex flex-col gap-1.5 px-2 pt-1',
editingConfig.disablePublicFileSharing && 'opacity-50'
)}
>
<span className='text-[var(--text-secondary)] text-xs'>
Auth modes public file-share links may use
</span>
<ChipDropdown
multiple
showAllOption={false}
allLabel='None'
value={fileShareAuthValue}
onChange={setFileShareAuthTypes}
options={FILE_SHARE_AUTH_TYPE_OPTIONS}
disabled={editingConfig.disablePublicFileSharing}
matchTriggerWidth={false}
className='w-[200px]'
/>
</div>
</div>
</SettingsSection>
</div>
)}
</SettingsPanel>
@@ -4,8 +4,13 @@
import { describe, expect, it } from 'vitest'
import {
createPermissionGroupBodySchema,
permissionGroupFullConfigSchema,
updatePermissionGroupBodySchema,
} from '@/lib/api/contracts/permission-groups'
import {
DEFAULT_PERMISSION_GROUP_CONFIG,
parsePermissionGroupConfig,
} from '@/lib/permission-groups/types'
describe('createPermissionGroupBodySchema', () => {
it('accepts a name-only body (scope is resolved and validated server-side)', () => {
@@ -80,3 +85,55 @@ describe('updatePermissionGroupBodySchema', () => {
expect(result.success).toBe(false)
})
})
/**
* The access-control group detail view decides whether its config buffer is
* dirty by comparing `JSON.stringify(savedConfig)` against
* `JSON.stringify(editingConfig)`, and reconciles the saved baseline from the
* update response. That only works while every config that reaches the client —
* from the list route and from the update route alike — carries the same key
* order, which holds because both pass through `parsePermissionGroupConfig` and
* then `permissionGroupFullConfigSchema`. If the two ever drift, the detail view
* would report unsaved changes forever after a successful save.
*/
describe('permissionGroupFullConfigSchema key order', () => {
it('matches parsePermissionGroupConfig so a saved config compares equal', () => {
const stored = parsePermissionGroupConfig({
allowedIntegrations: ['slack'],
hideCopilot: true,
})
const overWire = permissionGroupFullConfigSchema.parse(structuredClone(stored))
expect(JSON.stringify(overWire)).toBe(JSON.stringify(stored))
})
it('keeps an edited client buffer comparable to the server echo', () => {
const fromList = permissionGroupFullConfigSchema.parse(
structuredClone(parsePermissionGroupConfig(DEFAULT_PERMISSION_GROUP_CONFIG))
)
const edited = { ...fromList, hideDeployChatbot: true, deniedTools: ['slack_canvas'] }
const serverEcho = permissionGroupFullConfigSchema.parse(
structuredClone(parsePermissionGroupConfig(edited))
)
expect(JSON.stringify(serverEcho)).toBe(JSON.stringify(edited))
})
})
describe('permission group description trimming', () => {
it('trims a padded description on create', () => {
const result = createPermissionGroupBodySchema.safeParse({
name: 'Engineering',
description: ' padded ',
})
expect(result.success && result.data.description).toBe('padded')
})
it('trims a padded description on update', () => {
const result = updatePermissionGroupBodySchema.safeParse({ description: ' padded ' })
expect(result.success && result.data.description).toBe('padded')
})
it('still accepts a null description on update', () => {
const result = updatePermissionGroupBodySchema.safeParse({ description: null })
expect(result.success && result.data.description).toBeNull()
})
})
@@ -148,7 +148,7 @@ function refineWorkspaceScope(
export const createPermissionGroupBodySchema = z
.object({
name: z.string().trim().min(1).max(100),
description: z.string().max(500).optional(),
description: z.string().trim().max(500).optional(),
config: permissionGroupConfigSchema.optional(),
isDefault: z.boolean().optional(),
workspaceIds: workspaceIdsSchema.optional(),
@@ -158,7 +158,7 @@ export const createPermissionGroupBodySchema = z
export const updatePermissionGroupBodySchema = z
.object({
name: z.string().trim().min(1).max(100).optional(),
description: z.string().max(500).nullable().optional(),
description: z.string().trim().max(500).nullable().optional(),
config: permissionGroupConfigSchema.optional(),
isDefault: z.boolean().optional(),
workspaceIds: workspaceIdsSchema.optional(),
@@ -71,6 +71,20 @@ interface ChipDropdownBaseProps extends VariantProps<typeof chipVariants> {
leftIcon?: ChipIcon
/** Forwarded class for the trigger button. */
className?: string
/**
* Accessible name for the trigger. Use when the visible label sits outside
* the component (a field label above it) rather than in the selected value.
*/
'aria-label'?: string
/**
* Ids of the elements naming the trigger. Because `aria-labelledby` REPLACES
* the name derived from the trigger's contents, include the trigger's own id
* alongside the external label's — `\`${labelId} ${triggerId}\`` — or the
* selected value is dropped from the accessible name.
*/
'aria-labelledby'?: string
/** Id for the trigger button. Needed to reference it from `aria-labelledby`. */
id?: string
}
/**
@@ -168,6 +182,9 @@ const ChipDropdown = forwardRef<HTMLButtonElement, ChipDropdownProps>(
active,
fullWidth,
flush,
'aria-label': ariaLabel,
'aria-labelledby': ariaLabelledBy,
id,
} = props
const isMultiple = props.multiple === true
@@ -285,8 +302,11 @@ const ChipDropdown = forwardRef<HTMLButtonElement, ChipDropdownProps>(
<DropdownMenuTrigger asChild disabled={disabled}>
<button
ref={ref}
id={id}
type='button'
disabled={disabled}
aria-label={ariaLabel}
aria-labelledby={ariaLabelledBy}
className={cn(
chipVariants({ variant, active, fullWidth, flush }),
hasTriggerBorder && TRIGGER_BORDER_CLASS,