From e538a5676cee84b2ba66f18a522eea3b10fde8c0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 18:29:15 -0700 Subject: [PATCH 1/4] fix(skills): show the new skill after creating it The create page navigated to the new skill's detail route, but the unsaved-changes guard immediately undid it: `isDirty` was derived from `createSkill.isSuccess`, so on success the guard's effect fired `history.back()` to pop its sentinel entry and cancelled the navigation that had just run. The header stayed on "New skill" with the fields intact, and nothing confirmed the save. The guard now exposes `release()` to retire itself for the rest of the mount, and the create page calls it before navigating with `replace` so the sentinel entry is consumed rather than stacked. Also from a cleanup pass over the same surfaces: - `useCreateSkill` resolves the created row, so the page reads `created.id` instead of re-deriving it from the response list - seed the list cache with the upsert's authoritative list verbatim; the id-merge kept the previous ordering and appended the new skill last - treat placeholder data as loading in the detail page, which could otherwise flash "Skill not found." on a workspace switch - seed credential-detail drafts on id change rather than in a value-keyed effect, so a background refetch can't clobber an in-progress edit - toast on save and on delete failure, which were both silent --- .../hooks/use-credential-detail-form.ts | 15 +++++++---- .../hooks/use-unsaved-changes-guard.ts | 16 +++++++++-- .../skills/[skillId]/skill-detail.tsx | 15 +++++++++-- .../[workspaceId]/skills/new/skill-create.tsx | 19 ++++++------- apps/sim/hooks/queries/skills.ts | 27 ++++++++++--------- 5 files changed, 60 insertions(+), 32 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts index 7bd0e1375d0..d41ef467496 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts +++ b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts @@ -1,6 +1,6 @@ 'use client' -import { useCallback, useEffect, useState } from 'react' +import { useCallback, useState } from 'react' import { toast } from '@sim/emcn' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' @@ -30,11 +30,16 @@ export function useCredentialDetailForm({ const [displayNameDraft, setDisplayNameDraft] = useState('') const [descriptionDraft, setDescriptionDraft] = useState('') + const [prevCredentialId, setPrevCredentialId] = useState(null) - useEffect(() => { - setDisplayNameDraft(credential?.displayName ?? '') - setDescriptionDraft(credential?.description ?? '') - }, [credential?.id, credential?.displayName, credential?.description]) + // Seed drafts when the credential first resolves (or the route id changes); a + // background refetch of the same credential must not clobber an in-progress + // edit — nor silently reset isDirty, which pops the guard's history sentinel. + if (credential && credential.id !== prevCredentialId) { + setPrevCredentialId(credential.id) + setDisplayNameDraft(credential.displayName ?? '') + setDescriptionDraft(credential.description ?? '') + } const isDisplayNameDirty = credential ? displayNameDraft !== credential.displayName : false const isDescriptionDirty = credential 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..8c714c8289f 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,13 @@ 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(() => { + // Released: the caller is navigating away, so leave the seeded entry alone — + // popping it would cancel that navigation. + if (isReleased) return 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,7 +62,7 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG window.removeEventListener('beforeunload', handleBeforeUnload) window.removeEventListener('popstate', handlePopState) } - }, [isDirty]) + }, [isDirty, isReleased]) const handleBackClick = useCallback( (event: MouseEvent) => { @@ -75,5 +79,13 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG router.push(backHref) }, [router, backHref]) - return { showUnsavedAlert, setShowUnsavedAlert, handleBackClick, confirmDiscard } + /** + * Retires the guard for the rest of this mount: no unload warning, no Back + * trap, and no pop of the seeded entry when the form goes clean. Call it + * immediately before navigating away on a successful save, and navigate with + * `router.replace` so the seeded entry is the one consumed. + */ + const release = useCallback(() => setIsReleased(true), []) + + return { showUnsavedAlert, setShowUnsavedAlert, handleBackClick, confirmDiscard, release } } 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..27c71394e7e 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` can serve another workspace's list while this one loads + // (the key is workspace-scoped), which reads 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.') }) @@ -129,8 +134,14 @@ export function SkillDetail({ workspaceId, skillId }: SkillDetailProps) { setShowDeleteConfirm(false) try { await deleteSkill.mutateAsync({ workspaceId, skillId: skill.id }) - router.push(skillsHref) + // The skill is gone from the list cache, so this surface is about to have no + // entity to guard — retire it before navigating, as the create page does. + guard.release() + router.replace(skillsHref) } catch (error) { + toast.error("Couldn't delete skill", { + description: getErrorMessage(error, 'Please try again in a moment.'), + }) logger.error('Failed to delete skill', error) } } 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..18f2f019242 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,17 @@ 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}"`) + // Retire the guard before navigating — otherwise the form going clean pops + // its seeded history entry and lands us back on this create form. `replace` + // consumes that entry instead of 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/hooks/queries/skills.ts b/apps/sim/hooks/queries/skills.ts index 6817f6b6249..23e2b0b60ee 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, } /** @@ -88,17 +89,18 @@ 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. Resolve the created row here — by name, + // which is unique per workspace and which a same-named built-in is filtered + // out of — so consumers get an id instead of re-deriving one. + return { skills: data, created: data.find((skill) => skill.name === s.name) ?? null } }, - onSuccess: (data, variables) => { - 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()) - } - ) + onSuccess: ({ skills }, variables) => { + // The response is the same authoritative list GET /api/skills returns for + // this caller, so seed the cache with it verbatim. Merging by id would keep + // the previous ordering and append the new skill at the bottom until the + // refetch lands. + queryClient.setQueryData(skillsKeys.list(variables.workspaceId), skills) }, onSettled: (_data, _error, variables) => { queryClient.invalidateQueries({ queryKey: skillsKeys.list(variables.workspaceId) }) @@ -169,8 +171,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) }) }, }) } From 222838319a9b22fde57de1dbc0188bb55ca2ae6b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 19:00:47 -0700 Subject: [PATCH 2/4] improvement(skills): align the custom tools row and harden the guard lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups from an audit of the skills and custom tools surfaces. Custom tools row, now matching the skills row exactly: - the trailing arrow could be squeezed by a long description; the shared row owns `flex-shrink-0` for its trailing slot, so the five callers that hand-rolled it (and the two that forgot) all get it - drop a redundant `cursor-pointer` — Tailwind v3's preflight already sets it on `button` - the tile chrome was defined twice, in ResourceTile and again in SettingsResourceRow, despite ResourceTile's doc claiming to be the single source. Both now share RESOURCE_TILE_BASE/RESOURCE_TILE_FILL, and ResourceTile's redundant wrapper div is gone - skills' row uses the text-sm/text-caption tokens instead of literal pixels; the added gap-[1px] offsets the 21px->20px line-height so the row stays 39px Guard and cache correctness: - the delete path released the guard after the await, but the delete is optimistic — the row leaves the cache first, so the form went clean mid-flight and popped the sentinel before the release landed. Release up front and rearm if the request fails - hold the loading frame across the whole optimistic delete instead of flashing "Skill not found." on the way out - custom tools had the same placeholder-data bug just fixed in skill detail, and worse: a deep-linked id could resolve against the workspace just left - keep cached rows the create response omits, so a concurrent create isn't dropped until the refetch lands --- .../hooks/use-credential-detail-form.ts | 15 +++----- .../hooks/use-unsaved-changes-guard.ts | 24 +++++++++---- .../components/resource-tile/index.ts | 6 +++- .../resource-tile/resource-tile.tsx | 22 ++++++++---- .../components/byok/byok-key-manager.tsx | 4 +-- .../components/custom-tools/custom-tools.tsx | 7 ++-- .../recently-deleted/recently-deleted.tsx | 10 ++---- .../settings-resource-row.tsx | 18 ++++++---- .../skills/[skillId]/skill-detail.tsx | 18 ++++++---- .../[workspaceId]/skills/new/skill-create.tsx | 5 ++- .../workspace/[workspaceId]/skills/skills.tsx | 6 ++-- .../components/custom-blocks.tsx | 2 +- apps/sim/hooks/queries/custom-tools.ts | 2 +- apps/sim/hooks/queries/skills.ts | 36 ++++++++++--------- 14 files changed, 102 insertions(+), 73 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts index d41ef467496..7bd0e1375d0 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts +++ b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts @@ -1,6 +1,6 @@ 'use client' -import { useCallback, useState } from 'react' +import { useCallback, useEffect, useState } from 'react' import { toast } from '@sim/emcn' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' @@ -30,16 +30,11 @@ export function useCredentialDetailForm({ const [displayNameDraft, setDisplayNameDraft] = useState('') const [descriptionDraft, setDescriptionDraft] = useState('') - const [prevCredentialId, setPrevCredentialId] = useState(null) - // Seed drafts when the credential first resolves (or the route id changes); a - // background refetch of the same credential must not clobber an in-progress - // edit — nor silently reset isDirty, which pops the guard's history sentinel. - if (credential && credential.id !== prevCredentialId) { - setPrevCredentialId(credential.id) - setDisplayNameDraft(credential.displayName ?? '') - setDescriptionDraft(credential.description ?? '') - } + useEffect(() => { + setDisplayNameDraft(credential?.displayName ?? '') + setDescriptionDraft(credential?.description ?? '') + }, [credential?.id, credential?.displayName, credential?.description]) const isDisplayNameDirty = credential ? displayNameDraft !== credential.displayName : false const isDescriptionDirty = credential 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 8c714c8289f..d0a396f58df 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 @@ -30,8 +30,7 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG const hasSentinelRef = useRef(false) useEffect(() => { - // Released: the caller is navigating away, so leave the seeded entry alone — - // popping it would cancel that navigation. + // The caller is navigating away — popping the seeded entry would cancel it. if (isReleased) return if (!isDirty) { // Clean again while still mounted (saved/reverted): pop the seeded entry so @@ -80,12 +79,23 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG }, [router, backHref]) /** - * Retires the guard for the rest of this mount: no unload warning, no Back - * trap, and no pop of the seeded entry when the form goes clean. Call it - * immediately before navigating away on a successful save, and navigate with - * `router.replace` so the seeded entry is the one consumed. + * Retires the guard: no unload warning, no browser Back trap, 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), []) - return { showUnsavedAlert, setShowUnsavedAlert, handleBackClick, confirmDiscard, release } + /** 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 27c71394e7e..68597185bd4 100644 --- a/apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx +++ b/apps/sim/app/workspace/[workspaceId]/skills/[skillId]/skill-detail.tsx @@ -45,9 +45,9 @@ export function SkillDetail({ workspaceId, skillId }: SkillDetailProps) { const skillsHref = `/workspace/${workspaceId}/skills` const { data: skills = [], isPending, isPlaceholderData } = useSkills(workspaceId) - // `keepPreviousData` can serve another workspace's list while this one loads - // (the key is workspace-scoped), which reads as success with no match — treat - // that as loading so the detail never flashes "Skill not found." + // `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() @@ -132,13 +132,14 @@ 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 }) - // The skill is gone from the list cache, so this surface is about to have no - // entity to guard — retire it before navigating, as the create page does. - guard.release() router.replace(skillsHref) } catch (error) { + guard.rearm() toast.error("Couldn't delete skill", { description: getErrorMessage(error, 'Please try again in a moment.'), }) @@ -183,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 18f2f019242..5eaddfeee45 100644 --- a/apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx +++ b/apps/sim/app/workspace/[workspaceId]/skills/new/skill-create.tsx @@ -74,9 +74,8 @@ export function SkillCreate({ workspaceId }: SkillCreateProps) { }) setErrors({}) toast.success(`Created "${nameDraft}"`) - // Retire the guard before navigating — otherwise the form going clean pops - // its seeded history entry and lands us back on this create form. `replace` - // consumes that entry instead of stacking another. + // 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) { 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 23e2b0b60ee..f5e3c4e41c2 100644 --- a/apps/sim/hooks/queries/skills.ts +++ b/apps/sim/hooks/queries/skills.ts @@ -54,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: { @@ -68,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() @@ -90,17 +90,24 @@ export function useCreateSkill() { logger.info(`Created skill: ${s.name}`) // The upsert responds with the caller's whole skill list (built-ins - // included), not just the new row. Resolve the created row here — by name, - // which is unique per workspace and which a same-named built-in is filtered - // out of — so consumers get an id instead of re-deriving one. + // 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: ({ skills }, variables) => { - // The response is the same authoritative list GET /api/skills returns for - // this caller, so seed the cache with it verbatim. Merging by id would keep - // the previous ordering and append the new skill at the bottom until the - // refetch lands. - queryClient.setQueryData(skillsKeys.list(variables.workspaceId), skills) + // 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) => { + 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 + } + ) }, onSettled: (_data, _error, variables) => { queryClient.invalidateQueries({ queryKey: skillsKeys.list(variables.workspaceId) }) @@ -108,9 +115,6 @@ export function useCreateSkill() { }) } -/** - * Update skill mutation - */ interface UpdateSkillParams { workspaceId: string skillId: string From ee807563b889fb0aca38269646991033809aaf37 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 19:05:24 -0700 Subject: [PATCH 3/4] fix(skills): retire the in-app back guard on release too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit release() suppressed the unload warning and the browser Back trap, but the in-app back link still keyed only on isDirty — so the Skills chip could open the unsaved-changes modal after a successful create, while the drafts were still populated and the navigation was already in flight. --- .../hooks/use-unsaved-changes-guard.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) 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 d0a396f58df..47f048c92fe 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 @@ -65,12 +65,12 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG const handleBackClick = useCallback( (event: MouseEvent) => { - if (isDirty) { + if (isDirty && !isReleased) { event.preventDefault() setShowUnsavedAlert(true) } }, - [isDirty] + [isDirty, isReleased] ) const confirmDiscard = useCallback(() => { @@ -79,11 +79,12 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG }, [router, backHref]) /** - * Retires the guard: no unload warning, no browser Back trap, 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. + * 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), []) From 6aad77ee45758b396616672e4e3edffb5e840e2e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 24 Jul 2026 19:12:46 -0700 Subject: [PATCH 4/4] fix(skills): track a sentinel consumed while the guard is released MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit release() intentionally leaves the seeded history entry in place, but it also drops the popstate listener — so Back during the released window (an optimistic delete's round-trip) consumed that entry with nothing to record it. hasSentinelRef stayed true, and a failed delete's rearm() then skipped re-seeding, leaving the surface with no Back confirm despite unsaved edits. The released branch now keeps a bookkeeping-only popstate listener that clears the ref, so rearm() seeds a fresh entry when the old one is gone. --- .../hooks/use-unsaved-changes-guard.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) 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 47f048c92fe..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 @@ -30,8 +30,18 @@ export function useUnsavedChangesGuard({ isDirty, backHref }: UseUnsavedChangesG const hasSentinelRef = useRef(false) useEffect(() => { - // The caller is navigating away — popping the seeded entry would cancel it. - if (isReleased) return + // 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,