fix(skill): address code review issues for tab management

1. Add confirmation dialog when closing dirty tabs
2. Fix file double-click race condition with useDelayedClick hook
3. Fix previewTabId orphan state in closeTab
4. Remove auto-pin on every keystroke (VS Code behavior)
5. Extract shared MenuItem component to eliminate duplication
6. Make nodeId optional when node is provided (reduce props drilling)

web/app/components/workflow/skill/editor-tabs.tsx

@@ -2,12 +2,16 @@
 
 import type { FC } from 'react'
 import * as React from 'react'
+import { useCallback, useState } from 'react'
+import { useTranslation } from 'react-i18next'
+import Confirm from '@/app/components/base/confirm'
 import { cn } from '@/utils/classnames'
 import EditorTabItem from './editor-tab-item'
 import { useSkillAssetNodeMap } from './hooks/use-skill-asset-tree'
 import { useSkillEditorStore, useSkillEditorStoreApi } from './store'
 
 const EditorTabs: FC = () => {
+  const { t } = useTranslation('workflow')
   const openTabIds = useSkillEditorStore(s => s.openTabIds)
   const activeTabId = useSkillEditorStore(s => s.activeTabId)
   const previewTabId = useSkillEditorStore(s => s.previewTabId)
@@ -15,50 +19,82 @@ const EditorTabs: FC = () => {
   const storeApi = useSkillEditorStoreApi()
   const { data: nodeMap } = useSkillAssetNodeMap()
 
-  const handleTabClick = (fileId: string) => {
+  const [pendingCloseId, setPendingCloseId] = useState<string | null>(null)
+
+  const handleTabClick = useCallback((fileId: string) => {
     storeApi.getState().activateTab(fileId)
-  }
+  }, [storeApi])
 
-  const handleTabDoubleClick = (fileId: string) => {
+  const handleTabDoubleClick = useCallback((fileId: string) => {
     storeApi.getState().pinTab(fileId)
-  }
+  }, [storeApi])
 
-  const handleTabClose = (fileId: string) => {
+  const closeTab = useCallback((fileId: string) => {
     storeApi.getState().closeTab(fileId)
     storeApi.getState().clearDraftContent(fileId)
-  }
+  }, [storeApi])
+
+  const handleTabClose = useCallback((fileId: string) => {
+    if (dirtyContents.has(fileId)) {
+      setPendingCloseId(fileId)
+      return
+    }
+    closeTab(fileId)
+  }, [dirtyContents, closeTab])
+
+  const handleConfirmClose = useCallback(() => {
+    if (pendingCloseId) {
+      closeTab(pendingCloseId)
+      setPendingCloseId(null)
+    }
+  }, [pendingCloseId, closeTab])
+
+  const handleCancelClose = useCallback(() => {
+    setPendingCloseId(null)
+  }, [])
 
   if (openTabIds.length === 0)
     return null
 
   return (
-    <div
+    <>
-      className={cn(
+      <div
-        'flex items-center overflow-hidden rounded-t-lg border-b border-components-panel-border-subtle bg-components-panel-bg-alt',
+        className={cn(
-      )}
+          'flex items-center overflow-hidden rounded-t-lg border-b border-components-panel-border-subtle bg-components-panel-bg-alt',
-    >
+        )}
-      {openTabIds.map((fileId) => {
+      >
-        const node = nodeMap?.get(fileId)
+        {openTabIds.map((fileId) => {
-        const name = node?.name ?? fileId
+          const node = nodeMap?.get(fileId)
-        const isActive = activeTabId === fileId
+          const name = node?.name ?? fileId
-        const isDirty = dirtyContents.has(fileId)
+          const isActive = activeTabId === fileId
-        const isPreview = previewTabId === fileId
+          const isDirty = dirtyContents.has(fileId)
+          const isPreview = previewTabId === fileId
 
-        return (
+          return (
-          <EditorTabItem
+            <EditorTabItem
-            key={fileId}
+              key={fileId}
-            fileId={fileId}
+              fileId={fileId}
-            name={name}
+              name={name}
-            isActive={isActive}
+              isActive={isActive}
-            isDirty={isDirty}
+              isDirty={isDirty}
-            isPreview={isPreview}
+              isPreview={isPreview}
-            onClick={handleTabClick}
+              onClick={handleTabClick}
-            onClose={handleTabClose}
+              onClose={handleTabClose}
-            onDoubleClick={handleTabDoubleClick}
+              onDoubleClick={handleTabDoubleClick}
-          />
+            />
-        )
+          )
-      })}
+        })}
-    </div>
+      </div>
+      <Confirm
+        isShow={pendingCloseId !== null}
+        type="warning"
+        title={t('skillSidebar.unsavedChanges.title')}
+        content={t('skillSidebar.unsavedChanges.content')}
+        confirmText={t('skillSidebar.unsavedChanges.confirmClose')}
+        onConfirm={handleConfirmClose}
+        onCancel={handleCancelClose}
+      />
+    </>
   )
 }
 

web/app/components/workflow/skill/file-tree/file-node-menu.tsx

@@ -12,34 +12,10 @@ import { useTranslation } from 'react-i18next'
 import Confirm from '@/app/components/base/confirm'
 import { cn } from '@/utils/classnames'
 import { useFileOperations } from '../hooks/use-file-operations'
-
+import MenuItem from './menu-item'
-type MenuItemProps = {
-  icon: React.ElementType
-  label: string
-  onClick: () => void
-  disabled?: boolean
-}
-
-const MenuItem: React.FC<MenuItemProps> = ({ icon: Icon, label, onClick, disabled }) => (
-  <button
-    type="button"
-    onClick={onClick}
-    disabled={disabled}
-    className={cn(
-      'flex w-full items-center gap-2 rounded-lg px-3 py-2',
-      'hover:bg-state-base-hover disabled:cursor-not-allowed disabled:opacity-50',
-      'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-components-input-border-active',
-    )}
-  >
-    <Icon className="size-4 text-text-tertiary" aria-hidden="true" />
-    <span className="system-sm-regular text-text-secondary">
-      {label}
-    </span>
-  </button>
-)
 
 type FileItemMenuProps = {
-  nodeId: string
+  nodeId?: string
   onClose: () => void
   className?: string
   treeRef?: React.RefObject<TreeApi<TreeNodeData> | null>

web/app/components/workflow/skill/file-tree/folder-node-menu.tsx

@@ -16,34 +16,10 @@ import { useTranslation } from 'react-i18next'
 import Confirm from '@/app/components/base/confirm'
 import { cn } from '@/utils/classnames'
 import { useFileOperations } from '../hooks/use-file-operations'
-
+import MenuItem from './menu-item'
-type MenuItemProps = {
-  icon: React.ElementType
-  label: string
-  onClick: () => void
-  disabled?: boolean
-}
-
-const MenuItem: React.FC<MenuItemProps> = ({ icon: Icon, label, onClick, disabled }) => (
-  <button
-    type="button"
-    onClick={onClick}
-    disabled={disabled}
-    className={cn(
-      'flex w-full items-center gap-2 rounded-lg px-3 py-2',
-      'hover:bg-state-base-hover disabled:cursor-not-allowed disabled:opacity-50',
-      'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-components-input-border-active',
-    )}
-  >
-    <Icon className="size-4 text-text-tertiary" aria-hidden="true" />
-    <span className="system-sm-regular text-text-secondary">
-      {label}
-    </span>
-  </button>
-)
 
 type FileOperationsMenuProps = {
-  nodeId: string
+  nodeId?: string
   onClose: () => void
   className?: string
   treeRef?: React.RefObject<TreeApi<TreeNodeData> | null>

web/app/components/workflow/skill/file-tree/menu-item.tsx

@@ -0,0 +1,32 @@
+'use client'
+
+import type { FC } from 'react'
+import * as React from 'react'
+import { cn } from '@/utils/classnames'
+
+export type MenuItemProps = {
+  icon: React.ElementType
+  label: string
+  onClick: () => void
+  disabled?: boolean
+}
+
+const MenuItem: FC<MenuItemProps> = ({ icon: Icon, label, onClick, disabled }) => (
+  <button
+    type="button"
+    onClick={onClick}
+    disabled={disabled}
+    className={cn(
+      'flex w-full items-center gap-2 rounded-lg px-3 py-2',
+      'hover:bg-state-base-hover disabled:cursor-not-allowed disabled:opacity-50',
+      'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-components-input-border-active',
+    )}
+  >
+    <Icon className="size-4 text-text-tertiary" aria-hidden="true" />
+    <span className="system-sm-regular text-text-secondary">
+      {label}
+    </span>
+  </button>
+)
+
+export default React.memo(MenuItem)

web/app/components/workflow/skill/file-tree/tree-node.tsx

@@ -15,6 +15,7 @@ import {
   PortalToFollowElemTrigger,
 } from '@/app/components/base/portal-to-follow-elem'
 import { cn } from '@/utils/classnames'
+import { useDelayedClick } from '../hooks/use-delayed-click'
 import { useSkillEditorStore, useSkillEditorStoreApi } from '../store'
 import { getFileIconType } from '../utils/file-utils'
 import FileNodeMenu from './file-node-menu'
@@ -40,13 +41,26 @@ const TreeNode = ({ node, style, dragHandle }: NodeRendererProps<TreeNodeData>)
     [node],
   )
 
+  const openFilePreview = useCallback(() => {
+    storeApi.getState().openTab(node.data.id, { pinned: false })
+  }, [node.data.id, storeApi])
+
+  const openFilePinned = useCallback(() => {
+    storeApi.getState().openTab(node.data.id, { pinned: true })
+  }, [node.data.id, storeApi])
+
+  const { handleClick: handleFileClick, handleDoubleClick: handleFileDoubleClick } = useDelayedClick({
+    onSingleClick: openFilePreview,
+    onDoubleClick: openFilePinned,
+  })
+
   const handleClick = (e: React.MouseEvent) => {
     e.stopPropagation()
     node.select()
     if (isFolder)
       throttledToggle()
     else
-      storeApi.getState().openTab(node.data.id, { pinned: false })
+      handleFileClick()
   }
 
   const handleDoubleClick = (e: React.MouseEvent) => {
@@ -54,7 +68,7 @@ const TreeNode = ({ node, style, dragHandle }: NodeRendererProps<TreeNodeData>)
     if (isFolder)
       throttledToggle()
     else
-      storeApi.getState().openTab(node.data.id, { pinned: true })
+      handleFileDoubleClick()
   }
 
   const handleToggle = (e: React.MouseEvent) => {
@@ -181,14 +195,12 @@ const TreeNode = ({ node, style, dragHandle }: NodeRendererProps<TreeNodeData>)
           {isFolder
             ? (
                 <FolderNodeMenu
-                  nodeId={node.data.id}
                   onClose={() => setShowDropdown(false)}
                   node={node}
                 />
               )
             : (
                 <FileNodeMenu
-                  nodeId={node.data.id}
                   onClose={() => setShowDropdown(false)}
                   node={node}
                 />

web/app/components/workflow/skill/hooks/use-delayed-click.ts

@@ -0,0 +1,40 @@
+import { useCallback, useRef } from 'react'
+
+type UseDelayedClickOptions = {
+  delay?: number
+  onSingleClick: () => void
+  onDoubleClick: () => void
+}
+
+/**
+ * Hook to distinguish between single-click and double-click events.
+ * Single-click is delayed to allow double-click detection.
+ * Double-click cancels any pending single-click.
+ */
+export function useDelayedClick({
+  delay = 200,
+  onSingleClick,
+  onDoubleClick,
+}: UseDelayedClickOptions) {
+  const timeoutRef = useRef<NodeJS.Timeout | null>(null)
+
+  const handleClick = useCallback(() => {
+    if (timeoutRef.current)
+      clearTimeout(timeoutRef.current)
+
+    timeoutRef.current = setTimeout(() => {
+      onSingleClick()
+      timeoutRef.current = null
+    }, delay)
+  }, [delay, onSingleClick])
+
+  const handleDoubleClick = useCallback(() => {
+    if (timeoutRef.current) {
+      clearTimeout(timeoutRef.current)
+      timeoutRef.current = null
+    }
+    onDoubleClick()
+  }, [onDoubleClick])
+
+  return { handleClick, handleDoubleClick }
+}

web/app/components/workflow/skill/hooks/use-file-operations.ts

@@ -16,18 +16,19 @@ import { getAllDescendantFileIds } from '../utils/tree-utils'
 import { useSkillAssetTreeData } from './use-skill-asset-tree'
 
 type UseFileOperationsOptions = {
-  nodeId: string
+  nodeId?: string
   onClose: () => void
   treeRef?: React.RefObject<TreeApi<TreeNodeData> | null>
   node?: NodeApi<TreeNodeData>
 }
 
 export function useFileOperations({
-  nodeId,
+  nodeId: explicitNodeId,
   onClose,
   treeRef,
   node,
 }: UseFileOperationsOptions) {
+  const nodeId = node?.data.id ?? explicitNodeId ?? ''
   const { t } = useTranslation('workflow')
   const fileInputRef = useRef<HTMLInputElement>(null)
   const folderInputRef = useRef<HTMLInputElement>(null)

web/app/components/workflow/skill/skill-doc-editor.tsx

@@ -69,7 +69,6 @@ const SkillDocEditor: FC = () => {
     if (!activeTabId || !isEditable)
       return
     storeApi.getState().setDraftContent(activeTabId, value ?? '')
-    storeApi.getState().pinTab(activeTabId)
   }, [activeTabId, isEditable, storeApi])
 
   const handleSave = useCallback(async () => {

web/app/components/workflow/skill/store/index.ts

@@ -77,10 +77,14 @@ export const createTabSlice: StateCreator<TabSliceShape> = (set, get) => ({
         newActiveTabId = null
     }
 
+    const newPreviewTabId = previewTabId === fileId
+      ? null
+      : (previewTabId && newOpenTabIds.includes(previewTabId) ? previewTabId : null)
+
     set({
       openTabIds: newOpenTabIds,
       activeTabId: newActiveTabId,
-      previewTabId: previewTabId === fileId ? null : previewTabId,
+      previewTabId: newPreviewTabId,
     })
   },
 

web/i18n/en-US/workflow.json

@@ -1031,6 +1031,9 @@
   "skillSidebar.newFolder": "New folder",
   "skillSidebar.searchPlaceholder": "Search files…",
   "skillSidebar.toggleFolder": "Toggle folder",
+  "skillSidebar.unsavedChanges.confirmClose": "Discard",
+  "skillSidebar.unsavedChanges.content": "You have unsaved changes. Do you want to discard them?",
+  "skillSidebar.unsavedChanges.title": "Unsaved changes",
   "skillSidebar.uploading": "Uploading…",
   "tabs.-": "Default",
   "tabs.addAll": "Add all",

web/i18n/zh-Hans/workflow.json

@@ -1023,6 +1023,9 @@
   "skillSidebar.menu.uploadFolder": "上传文件夹",
   "skillSidebar.newFolder": "新建文件夹",
   "skillSidebar.searchPlaceholder": "搜索文件...",
+  "skillSidebar.unsavedChanges.confirmClose": "放弃",
+  "skillSidebar.unsavedChanges.content": "您有未保存的更改，是否放弃？",
+  "skillSidebar.unsavedChanges.title": "未保存的更改",
   "skillSidebar.uploading": "上传中...",
   "tabs.-": "默认",
   "tabs.addAll": "添加全部",