mirror of
https://github.com/langgenius/dify.git
synced 2026-05-03 17:08:03 +08:00
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.
This commit is contained in:
@ -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
|
||||
|
||||
@ -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'
|
||||
|
||||
@ -51,6 +51,32 @@ type SkillSaveProviderProps = {
|
||||
children: React.ReactNode
|
||||
}
|
||||
|
||||
const normalizeMetadata = (
|
||||
rawMetadata: Record<string, unknown> | undefined,
|
||||
content: string,
|
||||
): Record<string, unknown> | undefined => {
|
||||
if (!rawMetadata || typeof rawMetadata !== 'object' || !('tools' in rawMetadata))
|
||||
return rawMetadata
|
||||
|
||||
const toolIds = extractToolConfigIds(content)
|
||||
const rawTools = (rawMetadata as Record<string, unknown>).tools
|
||||
if (!rawTools || typeof rawTools !== 'object')
|
||||
return rawMetadata
|
||||
|
||||
const entries = Object.entries(rawTools as Record<string, unknown>)
|
||||
const nextTools = entries.reduce<Record<string, unknown>>((acc, [id, value]) => {
|
||||
if (toolIds.has(id))
|
||||
acc[id] = value
|
||||
return acc
|
||||
}, {})
|
||||
const nextMetadata = { ...(rawMetadata as Record<string, unknown>) }
|
||||
if (Object.keys(nextTools).length > 0)
|
||||
nextMetadata.tools = nextTools
|
||||
else
|
||||
delete nextMetadata.tools
|
||||
return nextMetadata
|
||||
}
|
||||
|
||||
const SkillSaveContext = React.createContext<SkillSaveContextValue | null>(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<string, unknown>).tools
|
||||
if (rawTools && typeof rawTools === 'object') {
|
||||
const entries = Object.entries(rawTools as Record<string, unknown>)
|
||||
const nextTools = entries.reduce<Record<string, unknown>>((acc, [id, value]) => {
|
||||
if (toolIds.has(id))
|
||||
acc[id] = value
|
||||
return acc
|
||||
}, {})
|
||||
const nextMetadata = { ...(rawMetadata as Record<string, unknown>) }
|
||||
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)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user