diff --git a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-unsaved-changes-guard.ts b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-unsaved-changes-guard.ts index 7e3ac184867..1dd0bb241bc 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-unsaved-changes-guard.ts +++ b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-unsaved-changes-guard.ts @@ -26,9 +26,22 @@ interface UseUnsavedChangesGuardParams { export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesGuardParams) { const router = useRouter() const [showUnsavedAlert, setShowUnsavedAlert] = useState(false) + const [isReleased, setIsReleased] = useState(false) const hasSentinelRef = useRef(false) useEffect(() => { + // The caller is navigating away — popping the seeded entry would cancel it. But + // Back during that window consumes the entry with no listener left to re-push + // it, so track that: a later rearm() must seed a fresh one rather than trust a + // stale ref and leave the surface unguarded. + if (isReleased) { + if (!hasSentinelRef.current) return + const handleSentinelConsumed = () => { + hasSentinelRef.current = false + } + window.addEventListener('popstate', handleSentinelConsumed) + return () => window.removeEventListener('popstate', handleSentinelConsumed) + } if (!isDirty) { // Clean again while still mounted (saved/reverted): pop the seeded entry so // it can't pile up across edit/save cycles. This runs in the effect body, @@ -58,16 +71,16 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG window.removeEventListener('beforeunload', handleBeforeUnload) window.removeEventListener('popstate', handlePopState) } - }, [isDirty]) + }, [isDirty, isReleased]) const handleBackClick = useCallback( (event: MouseEvent) => { - if (isDirty) { + if (isDirty && !isReleased) { event.preventDefault() setShowUnsavedAlert(true) } }, - [isDirty] + [isDirty, isReleased] ) const confirmDiscard = useCallback(() => { @@ -75,5 +88,25 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG router.push(backHref) }, [router, backHref]) - return { showUnsavedAlert, setShowUnsavedAlert, handleBackClick, confirmDiscard } + /** + * Retires the guard: no unload warning, no Back trap (browser or the in-app back + * link), and no pop of the seeded entry when the form goes clean. Call it before + * navigating away on a successful save, and navigate with `router.replace` so the + * seeded entry is the one consumed. An operation that goes clean before it + * resolves (an optimistic delete) must release up front and {@link rearm} if it + * fails. + */ + const release = useCallback(() => setIsReleased(true), []) + + /** Restores guarding after a released operation failed and the surface stays. */ + const rearm = useCallback(() => setIsReleased(false), []) + + return { + showUnsavedAlert, + setShowUnsavedAlert, + handleBackClick, + confirmDiscard, + release, + rearm, + } } diff --git a/apps/sim/app/workspace/[workspaceId]/components/resource-tile/index.ts b/apps/sim/app/workspace/[workspaceId]/components/resource-tile/index.ts index 507fe07a2f8..035cc143cdd 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/resource-tile/index.ts +++ b/apps/sim/app/workspace/[workspaceId]/components/resource-tile/index.ts @@ -1 +1,5 @@ -export { ResourceTile } from '@/app/workspace/[workspaceId]/components/resource-tile/resource-tile' +export { + RESOURCE_TILE_BASE, + RESOURCE_TILE_FILL, + ResourceTile, +} from '@/app/workspace/[workspaceId]/components/resource-tile/resource-tile' diff --git a/apps/sim/app/workspace/[workspaceId]/components/resource-tile/resource-tile.tsx b/apps/sim/app/workspace/[workspaceId]/components/resource-tile/resource-tile.tsx index 90907001430..91340e06517 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/resource-tile/resource-tile.tsx +++ b/apps/sim/app/workspace/[workspaceId]/components/resource-tile/resource-tile.tsx @@ -1,20 +1,30 @@ import type { ComponentType } from 'react' +import { cn } from '@sim/emcn' interface ResourceTileProps { icon: ComponentType<{ className?: string }> } +/** + * Geometry and border of the square resource tile — the single source for that + * chrome, shared by {@link ResourceTile} and `SettingsResourceRow` so the skills, + * custom tools, and settings surfaces cannot drift apart. Pair with a fill. Sizing + * the glyph is the tile's job: the descendant rule outranks an icon's own class. + */ +export const RESOURCE_TILE_BASE = + 'flex size-9 flex-shrink-0 items-center justify-center overflow-hidden rounded-xl border border-[var(--border-1)] [&_svg]:size-5' + +/** Filled treatment worn by the skills and custom tools resource tiles. */ +export const RESOURCE_TILE_FILL = 'bg-[var(--surface-4)] dark:bg-[var(--surface-5)]' + /** * Square glyph tile identifying a workspace resource — the leading visual on a - * resource's row and on its detail heading. Single source for that chrome so - * the skills and custom tools surfaces cannot drift apart. + * resource's row and on its detail heading. */ export function ResourceTile({ icon: Icon }: ResourceTileProps) { return ( -
-
- -
+
+
) } diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/byok/byok-key-manager.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/byok/byok-key-manager.tsx index ba957ffde7e..51d3bd630a8 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/byok/byok-key-manager.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/byok/byok-key-manager.tsx @@ -270,7 +270,7 @@ export function BYOKKeyManager(props: BYOKKeyManagerProps) { if (props.multiKey) { const keyCount = getProviderKeys(provider.id).length return ( -
+
{keyCount} {keyCount === 1 ? 'key' : 'keys'} @@ -283,7 +283,7 @@ export function BYOKKeyManager(props: BYOKKeyManagerProps) { if (readOnly) return null return ( -
+
openEditModal(provider.id)}>Update openDeleteConfirm(provider.id)}>Delete
diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/custom-tools/custom-tools.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/custom-tools/custom-tools.tsx index 95ce91a808f..1253c36e865 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/custom-tools/custom-tools.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/custom-tools/custom-tools.tsx @@ -26,7 +26,10 @@ export function CustomTools() { const workspacePermissions = useUserPermissionsContext() const canEdit = canMutateWorkspaceSettingsSection('custom-tools', workspacePermissions) - const { data: tools = [], isLoading, error } = useCustomTools(workspaceId) + const { data: tools = [], isPending, isPlaceholderData, error } = useCustomTools(workspaceId) + // Placeholder data is another workspace's tools reading as success — treat it as + // loading, or a deep-linked id resolves against the workspace the user just left. + const isLoading = isPending || isPlaceholderData const [searchTerm, setSearchTerm] = useSettingsSearch() const [selectedToolId, setSelectedToolId] = useQueryState(customToolIdParam.key, { @@ -119,7 +122,7 @@ export function CustomTools() { key={tool.id} type='button' onClick={() => void setSelectedToolId(tool.id)} - className='w-full cursor-pointer rounded-lg p-2 text-left transition-colors hover-hover:bg-[var(--surface-active)]' + className='w-full rounded-lg p-2 text-left transition-colors hover-hover:bg-[var(--surface-active)]' > } diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/recently-deleted/recently-deleted.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/recently-deleted/recently-deleted.tsx index 134f008f92e..f9c5a9a1914 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/recently-deleted/recently-deleted.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/recently-deleted/recently-deleted.tsx @@ -464,22 +464,18 @@ export function RecentlyDeleted() { } trailing={ !canRestore ? null : isRestoring ? ( - + Restoring... ) : isRestored ? ( -
+
Restored handleView(resource)}> View
) : ( - void handleRestore(resource)} - className='shrink-0' - > + void handleRestore(resource)}> Restore ) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/settings-resource-row/settings-resource-row.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/settings-resource-row/settings-resource-row.tsx index 7e31036010d..f5e9ea6c73b 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/settings-resource-row/settings-resource-row.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/settings-resource-row/settings-resource-row.tsx @@ -1,5 +1,9 @@ import type { ReactNode } from 'react' import { cn } from '@sim/emcn' +import { + RESOURCE_TILE_BASE, + RESOURCE_TILE_FILL, +} from '@/app/workspace/[workspaceId]/components/resource-tile' /** * The canonical settings "resource row": a rounded-bordered icon tile, a @@ -29,13 +33,13 @@ interface SettingsResourceRowProps { title: ReactNode /** Secondary muted line — truncates. */ description?: ReactNode - /** Trailing element pinned to the row's end (chips, actions menu, status). */ + /** + * Trailing element pinned to the row's end (chips, actions menu, status). The row + * keeps it at its natural size — callers never need their own `flex-shrink-0`. + */ trailing?: ReactNode } -const TILE_BASE = - 'flex size-9 flex-shrink-0 items-center justify-center overflow-hidden rounded-xl border border-[var(--border-1)] [&_svg]:size-5' - export function SettingsResourceRow({ icon, iconFill = false, @@ -49,8 +53,8 @@ export function SettingsResourceRow({
@@ -63,7 +67,7 @@ export function SettingsResourceRow({ )}
- {trailing} + {trailing ?
{trailing}
: null}
) } diff --git a/apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx b/apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx index c3189d4d50a..68597185bd4 100644 --- a/apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx +++ b/apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx @@ -44,7 +44,11 @@ export function SkillDetail({ workspaceId, skillId }: SkillDetailProps) { const router = useRouter() const skillsHref = `/workspace/${workspaceId}/skills` - const { data: skills = [], isPending: skillsLoading } = useSkills(workspaceId) + const { data: skills = [], isPending, isPlaceholderData } = useSkills(workspaceId) + // `keepPreviousData` carries the old cache entry across a workspace-id change, + // so another workspace's list can read as success with no match — treat that as + // loading so the detail never flashes "Skill not found." + const skillsLoading = isPending || isPlaceholderData const updateSkill = useUpdateSkill() const deleteSkill = useDeleteSkill() const skill = skills.find((s) => s.id === skillId) ?? null @@ -112,6 +116,7 @@ export function SkillDetail({ workspaceId, skillId }: SkillDetailProps) { }, }) setErrors({}) + toast.success(`Saved "${nameDraft}"`) } catch (error) { if (isSkillNameConflictError(error)) { setErrors({ name: getErrorMessage(error, 'This skill name is already taken.') }) @@ -127,10 +132,17 @@ export function SkillDetail({ workspaceId, skillId }: SkillDetailProps) { const handleConfirmDelete = async () => { if (!skill) return setShowDeleteConfirm(false) + // Optimistic: the skill leaves the list cache before the request resolves, so + // this surface goes clean mid-flight — release up front, rearm if it fails. + guard.release() try { await deleteSkill.mutateAsync({ workspaceId, skillId: skill.id }) - router.push(skillsHref) + router.replace(skillsHref) } catch (error) { + guard.rearm() + toast.error("Couldn't delete skill", { + description: getErrorMessage(error, 'Please try again in a moment.'), + }) logger.error('Failed to delete skill', error) } } @@ -172,7 +184,10 @@ export function SkillDetail({ workspaceId, skillId }: SkillDetailProps) { ) : null - if (skillsLoading && !skill) { + // A delete is optimistic and settles before `router.replace` commits, so the row + // is gone for a few frames while this surface is still mounted — hold the loading + // frame through both phases instead of flashing "Skill not found." on the way out. + if ((skillsLoading || deleteSkill.isPending || deleteSkill.isSuccess) && !skill) { return (

Loading…

diff --git a/apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx b/apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx index 928862b063b..5eaddfeee45 100644 --- a/apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx +++ b/apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx @@ -50,11 +50,7 @@ export function SkillCreate({ workspaceId }: SkillCreateProps) { const [contentSeed, setContentSeed] = useState(0) const [errors, setErrors] = useState({}) - // Drops on success so the guard pops its history sentinel before we navigate — - // otherwise Back from the new skill lands on a stale, empty create form. - const isDirty = - !createSkill.isSuccess && - (!!nameDraft.trim() || !!descriptionDraft.trim() || !!contentDraft.trim()) + const isDirty = !!nameDraft.trim() || !!descriptionDraft.trim() || !!contentDraft.trim() const guard = useUnsavedChangesGuard({ isDirty, backHref: skillsHref }) @@ -72,16 +68,16 @@ export function SkillCreate({ workspaceId }: SkillCreateProps) { } try { - const created = await createSkill.mutateAsync({ + const { created } = await createSkill.mutateAsync({ workspaceId, skill: { name: nameDraft, description: descriptionDraft, content: contentDraft }, }) setErrors({}) - // The upsert responds with the caller's whole skill list (built-ins - // included), not just the new row — match by name, which is unique per - // workspace, rather than trusting the first element. - const createdId = created.find((skill) => skill.name === nameDraft)?.id - router.push(createdId ? `${skillsHref}/${createdId}` : skillsHref) + toast.success(`Created "${nameDraft}"`) + // Detach the guard so its Back trap can't fire mid-navigation; `replace` then + // consumes the seeded entry rather than stacking another. + guard.release() + router.replace(created ? `${skillsHref}/${created.id}` : skillsHref) } catch (error) { if (isSkillNameConflictError(error)) { setErrors({ name: getErrorMessage(error, 'This skill name is already taken.') }) diff --git a/apps/sim/app/workspace/[workspaceId]/skills/skills.tsx b/apps/sim/app/workspace/[workspaceId]/skills/skills.tsx index 7004a6c3ebf..2f2dd9ad2ff 100644 --- a/apps/sim/app/workspace/[workspaceId]/skills/skills.tsx +++ b/apps/sim/app/workspace/[workspaceId]/skills/skills.tsx @@ -34,10 +34,10 @@ function SkillItem({ name, description, onClick }: SkillItemProps) { className='flex items-center gap-2.5 rounded-lg p-2 text-left transition-colors hover-hover:bg-[var(--surface-active)]' > -
- {name} +
+ {name} {description && ( - {description} + {description} )}
diff --git a/apps/sim/ee/custom-blocks/components/custom-blocks.tsx b/apps/sim/ee/custom-blocks/components/custom-blocks.tsx index 58301485c88..50d0b7ebceb 100644 --- a/apps/sim/ee/custom-blocks/components/custom-blocks.tsx +++ b/apps/sim/ee/custom-blocks/components/custom-blocks.tsx @@ -143,7 +143,7 @@ export function CustomBlocks() { title={cb.name} description={cb.description || undefined} trailing={ -
+
{!cb.enabled && Disabled} {canAdmin && }
diff --git a/apps/sim/hooks/queries/custom-tools.ts b/apps/sim/hooks/queries/custom-tools.ts index 5af4a5166a3..dfcc734731c 100644 --- a/apps/sim/hooks/queries/custom-tools.ts +++ b/apps/sim/hooks/queries/custom-tools.ts @@ -172,7 +172,7 @@ export function useCustomTools(workspaceId: string) { queryKey: customToolsKeys.list(workspaceId), queryFn: ({ signal }) => fetchCustomTools(workspaceId, signal), enabled: !!workspaceId, - staleTime: CUSTOM_TOOL_LIST_STALE_TIME, // 1 minute - tools don't change frequently + staleTime: CUSTOM_TOOL_LIST_STALE_TIME, placeholderData: keepPreviousData, }) } diff --git a/apps/sim/hooks/queries/skills.ts b/apps/sim/hooks/queries/skills.ts index 6817f6b6249..f5e3c4e41c2 100644 --- a/apps/sim/hooks/queries/skills.ts +++ b/apps/sim/hooks/queries/skills.ts @@ -26,7 +26,8 @@ export const skillsKeys = { all: ['skills'] as const, lists: () => [...skillsKeys.all, 'list'] as const, list: (workspaceId: string) => [...skillsKeys.lists(), workspaceId] as const, - members: (skillId?: string) => [...skillsKeys.all, 'members', skillId ?? ''] as const, + memberLists: () => [...skillsKeys.all, 'members'] as const, + members: (skillId?: string) => [...skillsKeys.memberLists(), skillId ?? ''] as const, } /** @@ -53,11 +54,6 @@ export function useSkills(workspaceId: string) { }) } -/** - * Create skill mutation. On success the created skill is merged into the list - * cache so consumers (e.g. the integration detail page's "Added" state) reflect - * it immediately, before the invalidation refetch lands. - */ interface CreateSkillParams { workspaceId: string skill: { @@ -67,6 +63,11 @@ interface CreateSkillParams { } } +/** + * Create skill mutation. Resolves to the caller's full skill list plus the newly + * created row, and seeds the list cache so consumers (e.g. the integration detail + * page's "Added" state) reflect it before the invalidation refetch lands. + */ export function useCreateSkill() { const queryClient = useQueryClient() @@ -88,15 +89,23 @@ export function useCreateSkill() { }) logger.info(`Created skill: ${s.name}`) - return data + // The upsert responds with the caller's whole skill list (built-ins + // included), not just the new row. Match by name — unique per workspace, + // and a same-named built-in is filtered out of the response. + return { skills: data, created: data.find((skill) => skill.name === s.name) ?? null } }, - onSuccess: (data, variables) => { + onSuccess: ({ skills }, variables) => { + // The response is the same authoritative list GET /api/skills returns for this + // caller, so its ordering wins. Cached rows absent from it (a concurrent create, + // or a delete this response post-dates) are kept rather than dropped — the two + // are indistinguishable here; the refetch settles both. queryClient.setQueryData( skillsKeys.list(variables.workspaceId), (prev) => { - const byId = new Map((prev ?? []).map((skill) => [skill.id, skill])) - for (const skill of data) byId.set(skill.id, skill) - return Array.from(byId.values()) + if (!prev) return skills + const responded = new Set(skills.map((skill) => skill.id)) + const missing = prev.filter((skill) => !responded.has(skill.id)) + return missing.length > 0 ? [...skills, ...missing] : skills } ) }, @@ -106,9 +115,6 @@ export function useCreateSkill() { }) } -/** - * Update skill mutation - */ interface UpdateSkillParams { workspaceId: string skillId: string @@ -169,8 +175,9 @@ export function useUpdateSkill() { } }, onSettled: (_data, _error, variables) => { + // Only name/description/content go over the wire here, none of which can + // change the editor roster — no need to invalidate it. queryClient.invalidateQueries({ queryKey: skillsKeys.list(variables.workspaceId) }) - queryClient.invalidateQueries({ queryKey: skillsKeys.members(variables.skillId) }) }, }) }