improvement(skills): align the custom tools row and harden the guard lifecycle

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

Author: waleedlatif1

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<string | null>(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

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,
+  }
 }

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'

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 (
-    <div className='size-9 flex-shrink-0'>
-      <div className='flex size-full items-center justify-center rounded-xl border border-[var(--border-1)] bg-[var(--surface-4)] dark:bg-[var(--surface-5)]'>
-        <Icon className='size-5 text-[var(--text-icon)]' />
-      </div>
+    <div className={cn(RESOURCE_TILE_BASE, RESOURCE_TILE_FILL)}>
+      <Icon className='text-[var(--text-icon)]' />
     </div>
   )
 }

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 (
-        <div className='flex flex-shrink-0 items-center gap-2'>
+        <div className='flex items-center gap-2'>
           <span className='text-[var(--text-muted)] text-caption'>
             {keyCount} {keyCount === 1 ? 'key' : 'keys'}
           </span>
@@ -283,7 +283,7 @@ export function BYOKKeyManager(props: BYOKKeyManagerProps) {
 
     if (readOnly) return null
     return (
-      <div className='flex flex-shrink-0 items-center gap-2'>
+      <div className='flex items-center gap-2'>
         <Chip onClick={() => openEditModal(provider.id)}>Update</Chip>
         <Chip onClick={() => openDeleteConfirm(provider.id)}>Delete</Chip>
       </div>

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)]'
             >
               <SettingsResourceRow
                 icon={<Wrench className='text-[var(--text-icon)]' />}

apps/sim/app/workspace/[workspaceId]/settings/components/recently-deleted/recently-deleted.tsx

@@ -464,22 +464,18 @@ export function RecentlyDeleted() {
                 }
                 trailing={
                   !canRestore ? null : isRestoring ? (
-                    <Chip variant='primary' disabled className='shrink-0'>
+                    <Chip variant='primary' disabled>
                       Restoring...
                     </Chip>
                   ) : isRestored ? (
-                    <div className='flex shrink-0 items-center gap-2'>
+                    <div className='flex items-center gap-2'>
                       <span className='text-[var(--text-muted)] text-small'>Restored</span>
                       <Chip variant='primary' onClick={() => handleView(resource)}>
                         View
                       </Chip>
                     </div>
                   ) : (
-                    <Chip
-                      variant='primary'
-                      onClick={() => void handleRestore(resource)}
-                      className='shrink-0'
-                    >
+                    <Chip variant='primary' onClick={() => void handleRestore(resource)}>
                       Restore
                     </Chip>
                   )

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({
       <div className='flex min-w-0 items-center gap-2.5'>
         <div
           className={cn(
-            TILE_BASE,
-            iconFilled ? 'bg-[var(--surface-4)] dark:bg-[var(--surface-5)]' : 'bg-[var(--bg)]',
+            RESOURCE_TILE_BASE,
+            iconFilled ? RESOURCE_TILE_FILL : 'bg-[var(--bg)]',
             iconFill ? '[&_img]:size-full' : '[&_img]:size-5'
           )}
         >
@@ -63,7 +67,7 @@ export function SettingsResourceRow({
           )}
         </div>
       </div>
-      {trailing}
+      {trailing ? <div className='flex flex-shrink-0 items-center'>{trailing}</div> : null}
     </div>
   )
 }

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 (
       <CredentialDetailLayout back={back} actions={actions}>
         <p className='py-12 text-center text-[var(--text-muted)] text-sm'>Loading…</p>

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) {