From a6a1ac4fa6a806910d702944627f18895bb9a434 Mon Sep 17 00:00:00 2001 From: yyh Date: Tue, 27 Jan 2026 13:02:55 +0800 Subject: [PATCH] fix(skill): prevent infinite save loop caused by unstable saveFile reference Use useRef to store saveFile reference and remove it from useEffect dependencies to prevent cleanup from re-triggering on reference changes. Also normalize metadata before comparison when clearing dirty state to ensure filtered tools match correctly. --- .../workflow/skill/file-content-panel.tsx | 7 ++- .../hooks/use-skill-save-manager.spec.tsx | 27 ++++++++++ .../skill/hooks/use-skill-save-manager.tsx | 49 +++++++++++-------- 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/web/app/components/workflow/skill/file-content-panel.tsx b/web/app/components/workflow/skill/file-content-panel.tsx index ad87d7ae80..39e95a455f 100644 --- a/web/app/components/workflow/skill/file-content-panel.tsx +++ b/web/app/components/workflow/skill/file-content-panel.tsx @@ -101,6 +101,9 @@ const FileContentPanel: FC = () => { const { saveFile, registerFallback, unregisterFallback } = useSkillSaveManager() + const saveFileRef = useRef(saveFile) + saveFileRef.current = saveFile + const fallbackRef = useRef({ content: originalContent, metadata: currentMetadata }) useEffect(() => { @@ -122,12 +125,12 @@ const FileContentPanel: FC = () => { return () => { const { content: fallbackContent, metadata: fallbackMetadata } = fallbackRef.current - void saveFile(fileTabId, { + void saveFileRef.current(fileTabId, { fallbackContent, fallbackMetadata, }) } - }, [fileTabId, isEditable, saveFile]) + }, [fileTabId, isEditable]) const handleEditorDidMount: OnMount = useCallback((editor, monaco) => { editorRef.current = editor diff --git a/web/app/components/workflow/skill/hooks/use-skill-save-manager.spec.tsx b/web/app/components/workflow/skill/hooks/use-skill-save-manager.spec.tsx index ef7cfd71d3..14ab8dfc09 100644 --- a/web/app/components/workflow/skill/hooks/use-skill-save-manager.spec.tsx +++ b/web/app/components/workflow/skill/hooks/use-skill-save-manager.spec.tsx @@ -201,6 +201,33 @@ describe('useSkillSaveManager', () => { }) }) + it('should clear dirty metadata when filtered tools match saved snapshot', async () => { + // Arrange + const appId = 'app-1' + const fileId = 'file-1' + const toolId1 = '00000000-0000-0000-0000-000000000001' + const toolId2 = '00000000-0000-0000-0000-000000000002' + const content = `Hello §[tool].[provider].[tool-name].[${toolId1}]§` + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftMetadata(fileId, { + tools: { + [toolId1]: { type: 'builtin' }, + [toolId2]: { type: 'builtin' }, + }, + }) + setCachedContent(queryClient, appId, fileId, JSON.stringify({ content })) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const response = await result.current.saveFile(fileId) + + // Assert + expect(response.saved).toBe(true) + expect(store.getState().dirtyMetadataIds.has(fileId)).toBe(false) + }) + it('should return unsaved when metadata is dirty but no content is available', async () => { // Arrange const appId = 'app-1' diff --git a/web/app/components/workflow/skill/hooks/use-skill-save-manager.tsx b/web/app/components/workflow/skill/hooks/use-skill-save-manager.tsx index b488f5bc24..6f751520bd 100644 --- a/web/app/components/workflow/skill/hooks/use-skill-save-manager.tsx +++ b/web/app/components/workflow/skill/hooks/use-skill-save-manager.tsx @@ -51,6 +51,32 @@ type SkillSaveProviderProps = { children: React.ReactNode } +const normalizeMetadata = ( + rawMetadata: Record | undefined, + content: string, +): Record | undefined => { + if (!rawMetadata || typeof rawMetadata !== 'object' || !('tools' in rawMetadata)) + return rawMetadata + + const toolIds = extractToolConfigIds(content) + const rawTools = (rawMetadata as Record).tools + if (!rawTools || typeof rawTools !== 'object') + return rawMetadata + + const entries = Object.entries(rawTools as Record) + const nextTools = entries.reduce>((acc, [id, value]) => { + if (toolIds.has(id)) + acc[id] = value + return acc + }, {}) + const nextMetadata = { ...(rawMetadata as Record) } + if (Object.keys(nextTools).length > 0) + nextMetadata.tools = nextTools + else + delete nextMetadata.tools + return nextMetadata +} + const SkillSaveContext = React.createContext(null) export const SkillSaveProvider = ({ @@ -109,25 +135,7 @@ export const SkillSaveProvider = ({ if (content === undefined) return null - let metadata = rawMetadata - if (rawMetadata && typeof rawMetadata === 'object' && 'tools' in rawMetadata) { - const toolIds = extractToolConfigIds(content) - const rawTools = (rawMetadata as Record).tools - if (rawTools && typeof rawTools === 'object') { - const entries = Object.entries(rawTools as Record) - const nextTools = entries.reduce>((acc, [id, value]) => { - if (toolIds.has(id)) - acc[id] = value - return acc - }, {}) - const nextMetadata = { ...(rawMetadata as Record) } - if (Object.keys(nextTools).length > 0) - nextMetadata.tools = nextTools - else - delete nextMetadata.tools - metadata = nextMetadata - } - } + const metadata = normalizeMetadata(rawMetadata, content) return { content, @@ -189,7 +197,8 @@ export const SkillSaveProvider = ({ if (snapshot.hasMetadataDirty) { const latestMetadata = latestState.fileMetadata.get(fileId) - if (isDeepEqual(latestMetadata, snapshot.metadata)) + const normalizedLatest = normalizeMetadata(latestMetadata, snapshot.content) + if (isDeepEqual(normalizedLatest, snapshot.metadata)) latestState.clearDraftMetadata(fileId) }