From b305abdc8f5dfefe54fea34d755a22fd21b94a25 Mon Sep 17 00:00:00 2001 From: yyh Date: Sun, 25 Jan 2026 19:50:34 +0800 Subject: [PATCH] fix(skill-editor): align autosave fallbacks - use cleanup-based save on tab switch with stable fallback snapshots - add fallback registry for metadata-only autosave consistency - add autosave/save-manager tests --- .../workflow/skill/file-content-panel.tsx | 30 ++ .../skill/hooks/use-skill-auto-save.spec.tsx | 80 ++++ .../skill/hooks/use-skill-auto-save.ts | 21 +- .../skill/hooks/use-skill-file-save.spec.tsx | 209 ++++++++++ .../hooks/use-skill-save-manager.spec.tsx | 365 ++++++++++++++++++ .../skill/hooks/use-skill-save-manager.tsx | 25 +- web/app/components/workflow/skill/main.tsx | 6 +- 7 files changed, 709 insertions(+), 27 deletions(-) create mode 100644 web/app/components/workflow/skill/hooks/use-skill-auto-save.spec.tsx create mode 100644 web/app/components/workflow/skill/hooks/use-skill-file-save.spec.tsx create mode 100644 web/app/components/workflow/skill/hooks/use-skill-save-manager.spec.tsx diff --git a/web/app/components/workflow/skill/file-content-panel.tsx b/web/app/components/workflow/skill/file-content-panel.tsx index 7aa148d311..944d4a8f45 100644 --- a/web/app/components/workflow/skill/file-content-panel.tsx +++ b/web/app/components/workflow/skill/file-content-panel.tsx @@ -20,6 +20,7 @@ import { useFileTypeInfo } from './hooks/use-file-type-info' import { useSkillAssetNodeMap } from './hooks/use-skill-asset-tree' import { useSkillFileData } from './hooks/use-skill-file-data' import { useSkillFileSave } from './hooks/use-skill-file-save' +import { useSkillSaveManager } from './hooks/use-skill-save-manager' import StartTabContent from './start-tab' import { getFileLanguage } from './utils/file-utils' import MediaFilePreview from './viewer/media-file-preview' @@ -108,6 +109,35 @@ const FileContentPanel: FC = () => { t, }) + const { saveFile, registerFallback, unregisterFallback } = useSkillSaveManager() + + const fallbackRef = useRef({ content: originalContent, metadata: currentMetadata }) + fallbackRef.current = { content: originalContent, metadata: currentMetadata } + + useEffect(() => { + if (!fileTabId || fileContent?.content === undefined) + return + + registerFallback(fileTabId, { content: originalContent, metadata: currentMetadata }) + + return () => { + unregisterFallback(fileTabId) + } + }, [fileTabId, fileContent?.content, originalContent, currentMetadata, registerFallback, unregisterFallback]) + + useEffect(() => { + if (!fileTabId || !isEditable) + return + + const { content: fallbackContent, metadata: fallbackMetadata } = fallbackRef.current + return () => { + void saveFile(fileTabId, { + fallbackContent, + fallbackMetadata, + }) + } + }, [fileTabId, isEditable, saveFile]) + const handleEditorDidMount: OnMount = useCallback((editor, monaco) => { editorRef.current = editor monaco.editor.setTheme(appTheme === Theme.light ? 'light' : 'vs-dark') diff --git a/web/app/components/workflow/skill/hooks/use-skill-auto-save.spec.tsx b/web/app/components/workflow/skill/hooks/use-skill-auto-save.spec.tsx new file mode 100644 index 0000000000..71a97e5001 --- /dev/null +++ b/web/app/components/workflow/skill/hooks/use-skill-auto-save.spec.tsx @@ -0,0 +1,80 @@ +import { act, renderHook } from '@testing-library/react' +import { useSkillAutoSave } from './use-skill-auto-save' + +const { mockSaveAllDirty } = vi.hoisted(() => ({ + mockSaveAllDirty: vi.fn(), +})) + +vi.mock('./use-skill-save-manager', () => ({ + useSkillSaveManager: () => ({ + saveAllDirty: mockSaveAllDirty, + }), +})) + +const setVisibilityState = (state: DocumentVisibilityState) => { + Object.defineProperty(document, 'visibilityState', { + configurable: true, + value: state, + }) +} + +describe('useSkillAutoSave', () => { + beforeEach(() => { + vi.clearAllMocks() + setVisibilityState('visible') + }) + + it('should save all dirty files on unmount', () => { + const { unmount } = renderHook(() => useSkillAutoSave()) + + unmount() + + expect(mockSaveAllDirty).toHaveBeenCalledTimes(1) + }) + + it('should save all dirty files when document becomes hidden', () => { + renderHook(() => useSkillAutoSave()) + + setVisibilityState('hidden') + act(() => { + document.dispatchEvent(new Event('visibilitychange')) + }) + + expect(mockSaveAllDirty).toHaveBeenCalledTimes(1) + }) + + it('should not save when document becomes visible', () => { + renderHook(() => useSkillAutoSave()) + + setVisibilityState('visible') + act(() => { + document.dispatchEvent(new Event('visibilitychange')) + }) + + expect(mockSaveAllDirty).not.toHaveBeenCalled() + }) + + it('should save all dirty files before unload', () => { + renderHook(() => useSkillAutoSave()) + + act(() => { + window.dispatchEvent(new Event('beforeunload')) + }) + + expect(mockSaveAllDirty).toHaveBeenCalledTimes(1) + }) + + it('should remove listeners after unmount', () => { + const { unmount } = renderHook(() => useSkillAutoSave()) + + unmount() + + setVisibilityState('hidden') + act(() => { + document.dispatchEvent(new Event('visibilitychange')) + window.dispatchEvent(new Event('beforeunload')) + }) + + expect(mockSaveAllDirty).toHaveBeenCalledTimes(1) + }) +}) diff --git a/web/app/components/workflow/skill/hooks/use-skill-auto-save.ts b/web/app/components/workflow/skill/hooks/use-skill-auto-save.ts index 09d091f098..a0407a0b0f 100644 --- a/web/app/components/workflow/skill/hooks/use-skill-auto-save.ts +++ b/web/app/components/workflow/skill/hooks/use-skill-auto-save.ts @@ -1,25 +1,8 @@ import { useEventListener, useUnmount } from 'ahooks' -import { useEffect, useRef } from 'react' -import { START_TAB_ID } from '../constants' import { useSkillSaveManager } from './use-skill-save-manager' -type UseSkillAutoSaveParams = { - activeTabId: string | null -} - -export function useSkillAutoSave({ - activeTabId, -}: UseSkillAutoSaveParams): void { - const { saveFile, saveAllDirty } = useSkillSaveManager() - const prevActiveTabIdRef = useRef(activeTabId) - - useEffect(() => { - const prevActiveTabId = prevActiveTabIdRef.current - if (prevActiveTabId && prevActiveTabId !== activeTabId && prevActiveTabId !== START_TAB_ID) - void saveFile(prevActiveTabId) - - prevActiveTabIdRef.current = activeTabId - }, [activeTabId, saveFile]) +export function useSkillAutoSave(): void { + const { saveAllDirty } = useSkillSaveManager() useUnmount(() => { saveAllDirty() diff --git a/web/app/components/workflow/skill/hooks/use-skill-file-save.spec.tsx b/web/app/components/workflow/skill/hooks/use-skill-file-save.spec.tsx new file mode 100644 index 0000000000..ed6d992c2f --- /dev/null +++ b/web/app/components/workflow/skill/hooks/use-skill-file-save.spec.tsx @@ -0,0 +1,209 @@ +import type { SaveFileOptions, SaveResult } from './use-skill-save-manager' +import { act, renderHook, waitFor } from '@testing-library/react' +import Toast from '@/app/components/base/toast' +import { useSkillFileSave } from './use-skill-file-save' + +const { mockSaveFile, mockToastNotify } = vi.hoisted(() => ({ + mockSaveFile: vi.fn<(fileId: string, options?: SaveFileOptions) => Promise>(), + mockToastNotify: vi.fn(), +})) + +vi.mock('./use-skill-save-manager', () => ({ + useSkillSaveManager: () => ({ + saveFile: mockSaveFile, + }), +})) + +vi.mock('@/app/components/base/toast', () => ({ + default: { + notify: mockToastNotify, + }, +})) + +const createParams = (overrides: Partial[0]> = {}) => ({ + appId: 'app-1', + activeTabId: 'file-1' as string | null, + isEditable: true, + originalContent: 'original content', + currentMetadata: { version: 1 } as Record, + t: vi.fn(() => 'saved-message') as unknown as Parameters[0]['t'], + ...overrides, +}) + +// Scenario: save behavior and shortcut handling for skill file saves. +describe('useSkillFileSave', () => { + beforeEach(() => { + vi.clearAllMocks() + mockSaveFile.mockResolvedValue({ saved: false }) + }) + + // Scenario: guard clauses prevent invalid saves. + describe('Guards', () => { + it('should return early when active tab id is missing', async () => { + // Arrange + const params = createParams({ activeTabId: null }) + const { result } = renderHook(() => useSkillFileSave(params)) + + // Act + await act(async () => { + await result.current() + }) + + // Assert + expect(mockSaveFile).not.toHaveBeenCalled() + expect(Toast.notify).not.toHaveBeenCalled() + }) + + it('should return early when app id is empty', async () => { + // Arrange + const params = createParams({ appId: '' }) + const { result } = renderHook(() => useSkillFileSave(params)) + + // Act + await act(async () => { + await result.current() + }) + + // Assert + expect(mockSaveFile).not.toHaveBeenCalled() + expect(Toast.notify).not.toHaveBeenCalled() + }) + + it('should return early when not editable', async () => { + // Arrange + const params = createParams({ isEditable: false }) + const { result } = renderHook(() => useSkillFileSave(params)) + + // Act + await act(async () => { + await result.current() + }) + + // Assert + expect(mockSaveFile).not.toHaveBeenCalled() + expect(Toast.notify).not.toHaveBeenCalled() + }) + }) + + // Scenario: save results surface as toast notifications. + describe('Save Results', () => { + it('should call saveFile with fallback data when valid', async () => { + // Arrange + const params = createParams({ + originalContent: 'fallback content', + currentMetadata: { tag: 'v1' }, + }) + const { result } = renderHook(() => useSkillFileSave(params)) + + // Act + await act(async () => { + await result.current() + }) + + // Assert + expect(mockSaveFile).toHaveBeenCalledWith('file-1', { + fallbackContent: 'fallback content', + fallbackMetadata: { tag: 'v1' }, + }) + expect(Toast.notify).not.toHaveBeenCalled() + }) + + it('should show error toast when save fails', async () => { + // Arrange + const params = createParams() + mockSaveFile.mockResolvedValueOnce({ saved: false, error: new Error('boom') }) + const { result } = renderHook(() => useSkillFileSave(params)) + + // Act + await act(async () => { + await result.current() + }) + + // Assert + expect(Toast.notify).toHaveBeenCalledWith({ + type: 'error', + message: 'Error: boom', + }) + expect(params.t).not.toHaveBeenCalled() + }) + + it('should show success toast when save succeeds', async () => { + // Arrange + const params = createParams() + mockSaveFile.mockResolvedValueOnce({ saved: true }) + const { result } = renderHook(() => useSkillFileSave(params)) + + // Act + await act(async () => { + await result.current() + }) + + // Assert + expect(params.t).toHaveBeenCalledWith('api.saved', { ns: 'common' }) + expect(Toast.notify).toHaveBeenCalledWith({ + type: 'success', + message: 'saved-message', + }) + }) + }) + + // Scenario: Ctrl/Cmd+S triggers save and suppresses default behavior. + describe('Keyboard Shortcut', () => { + it('should trigger save on Ctrl+S', async () => { + // Arrange + const params = createParams() + renderHook(() => useSkillFileSave(params)) + const event = new KeyboardEvent('keydown', { key: 's', ctrlKey: true, cancelable: true }) + const preventDefault = vi.fn() + Object.defineProperty(event, 'preventDefault', { value: preventDefault }) + + // Act + act(() => { + window.dispatchEvent(event) + }) + + // Assert + await waitFor(() => { + expect(mockSaveFile).toHaveBeenCalled() + }) + expect(preventDefault).toHaveBeenCalled() + }) + + it('should trigger save on Cmd+S', async () => { + // Arrange + const params = createParams() + renderHook(() => useSkillFileSave(params)) + const event = new KeyboardEvent('keydown', { key: 's', metaKey: true, cancelable: true }) + const preventDefault = vi.fn() + Object.defineProperty(event, 'preventDefault', { value: preventDefault }) + + // Act + act(() => { + window.dispatchEvent(event) + }) + + // Assert + await waitFor(() => { + expect(mockSaveFile).toHaveBeenCalled() + }) + expect(preventDefault).toHaveBeenCalled() + }) + + it('should not trigger save when key is not s', async () => { + // Arrange + const params = createParams() + renderHook(() => useSkillFileSave(params)) + const event = new KeyboardEvent('keydown', { key: 'x', ctrlKey: true, cancelable: true }) + + // Act + act(() => { + window.dispatchEvent(event) + }) + + // Assert + await waitFor(() => { + expect(mockSaveFile).not.toHaveBeenCalled() + }) + }) + }) +}) 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 new file mode 100644 index 0000000000..07d5ae0e57 --- /dev/null +++ b/web/app/components/workflow/skill/hooks/use-skill-save-manager.spec.tsx @@ -0,0 +1,365 @@ +import type { ReactNode } from 'react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { renderHook, waitFor } from '@testing-library/react' +import { WorkflowContext } from '@/app/components/workflow/context' +import { createWorkflowStore } from '@/app/components/workflow/store' +import { consoleQuery } from '@/service/client' +import { START_TAB_ID } from '../constants' +import { SkillSaveProvider, useSkillSaveManager } from './use-skill-save-manager' + +const { mockMutateAsync } = vi.hoisted(() => ({ + mockMutateAsync: vi.fn(), +})) + +vi.mock('@/service/use-app-asset', () => ({ + useUpdateAppAssetFileContent: () => ({ + mutateAsync: mockMutateAsync, + }), +})) + +const createQueryClient = () => new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, +}) + +const createWrapper = (params: { appId: string, store: ReturnType, queryClient: QueryClient }) => { + const { appId, store, queryClient } = params + return ({ children }: { children: ReactNode }) => ( + + + + {children} + + + + ) +} + +const setCachedContent = (queryClient: QueryClient, appId: string, fileId: string, content: string, extra: Record = {}) => { + const queryKey = consoleQuery.appAsset.getFileContent.queryKey({ + input: { params: { appId, nodeId: fileId } }, + }) + queryClient.setQueryData(queryKey, { ...extra, content }) + return queryKey +} + +const getCachedPayload = (queryClient: QueryClient, appId: string, fileId: string) => { + const queryKey = consoleQuery.appAsset.getFileContent.queryKey({ + input: { params: { appId, nodeId: fileId } }, + }) + const cached = queryClient.getQueryData<{ content?: string }>(queryKey) + if (!cached?.content) + return null + return JSON.parse(cached.content) as Record +} + +// Scenario: skill save manager coordinates store state, cache, and mutations. +describe('useSkillSaveManager', () => { + beforeEach(() => { + vi.clearAllMocks() + mockMutateAsync.mockResolvedValue(undefined) + }) + + it('should throw when used outside provider', () => { + expect(() => renderHook(() => useSkillSaveManager())).toThrow('Missing SkillSaveProvider in the tree') + }) + + // Scenario: save guard clauses block invalid saves. + describe('Guards', () => { + it('should return unsaved when app id is missing', async () => { + // Arrange + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId: '', store, queryClient }) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const response = await result.current.saveFile('file-1') + + // Assert + expect(response.saved).toBe(false) + expect(mockMutateAsync).not.toHaveBeenCalled() + }) + + it('should return unsaved when no dirty content or metadata exists', async () => { + // Arrange + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId: 'app-1', store, queryClient }) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const response = await result.current.saveFile('file-1') + + // Assert + expect(response.saved).toBe(false) + expect(mockMutateAsync).not.toHaveBeenCalled() + }) + + it('should skip saves for the start tab', async () => { + // Arrange + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId: 'app-1', store, queryClient }) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const response = await result.current.saveFile(START_TAB_ID) + + // Assert + expect(response.saved).toBe(false) + expect(mockMutateAsync).not.toHaveBeenCalled() + }) + }) + + // Scenario: successful saves update cache and clear draft state. + describe('Saving', () => { + it('should save draft content, update cache, and clear draft content', async () => { + // Arrange + const appId = 'app-1' + const fileId = 'file-1' + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftContent(fileId, 'draft-content') + store.getState().setFileMetadata(fileId, { author: 'test' }) + const queryKey = setCachedContent(queryClient, appId, fileId, JSON.stringify({ content: 'old' }), { extra: 'keep' }) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const response = await result.current.saveFile(fileId) + + // Assert + expect(response.saved).toBe(true) + expect(mockMutateAsync).toHaveBeenCalledWith({ + appId, + nodeId: fileId, + payload: { content: 'draft-content', metadata: { author: 'test' } }, + }) + expect(store.getState().dirtyContents.has(fileId)).toBe(false) + expect(queryClient.getQueryData<{ extra?: string }>(queryKey)?.extra).toBe('keep') + expect(getCachedPayload(queryClient, appId, fileId)).toEqual({ + content: 'draft-content', + metadata: { author: 'test' }, + }) + }) + + it('should save metadata-only changes using cached json content', async () => { + // Arrange + const appId = 'app-1' + const fileId = 'file-1' + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftMetadata(fileId, { version: 2 }) + setCachedContent(queryClient, appId, fileId, JSON.stringify({ content: 'cached-content' })) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const response = await result.current.saveFile(fileId) + + // Assert + expect(response.saved).toBe(true) + expect(mockMutateAsync).toHaveBeenCalledWith({ + appId, + nodeId: fileId, + payload: { content: 'cached-content', metadata: { version: 2 } }, + }) + expect(store.getState().dirtyMetadataIds.has(fileId)).toBe(false) + }) + + it('should fall back to raw cached content when json parsing fails', async () => { + // Arrange + const appId = 'app-1' + const fileId = 'file-1' + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftMetadata(fileId, { version: 3 }) + setCachedContent(queryClient, appId, fileId, 'raw-content') + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const response = await result.current.saveFile(fileId) + + // Assert + expect(response.saved).toBe(true) + expect(mockMutateAsync).toHaveBeenCalledWith({ + appId, + nodeId: fileId, + payload: { content: 'raw-content', metadata: { version: 3 } }, + }) + }) + + it('should return unsaved when metadata is dirty but no content is available', async () => { + // Arrange + const appId = 'app-1' + const fileId = 'file-1' + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftMetadata(fileId, { version: 4 }) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const response = await result.current.saveFile(fileId) + + // Assert + expect(response.saved).toBe(false) + expect(mockMutateAsync).not.toHaveBeenCalled() + }) + + it('should keep drafts when mutation fails', async () => { + // Arrange + const appId = 'app-1' + const fileId = 'file-1' + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftContent(fileId, 'draft-content') + mockMutateAsync.mockRejectedValueOnce(new Error('failed')) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const response = await result.current.saveFile(fileId) + + // Assert + expect(response.saved).toBe(false) + expect(response.error).toBeInstanceOf(Error) + expect(store.getState().dirtyContents.has(fileId)).toBe(true) + }) + }) + + // Scenario: fallback registry supplies content/metadata when other sources are empty. + describe('Fallback Registry', () => { + it('should use registered fallback when cache and options are missing', async () => { + // Arrange + const appId = 'app-1' + const fileId = 'file-1' + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.setState({ + fileMetadata: new Map>(), + dirtyMetadataIds: new Set([fileId]), + }) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + result.current.registerFallback(fileId, { content: 'fallback-content', metadata: { source: 'registry' } }) + const response = await result.current.saveFile(fileId) + + // Assert + expect(response.saved).toBe(true) + expect(mockMutateAsync).toHaveBeenCalledWith({ + appId, + nodeId: fileId, + payload: { content: 'fallback-content', metadata: { source: 'registry' } }, + }) + }) + + it('should return unsaved after fallback is unregistered', async () => { + // Arrange + const appId = 'app-1' + const fileId = 'file-1' + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.setState({ + fileMetadata: new Map>(), + dirtyMetadataIds: new Set([fileId]), + }) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + result.current.registerFallback(fileId, { content: 'fallback-content', metadata: { source: 'registry' } }) + result.current.unregisterFallback(fileId) + const response = await result.current.saveFile(fileId) + + // Assert + expect(response.saved).toBe(false) + expect(mockMutateAsync).not.toHaveBeenCalled() + }) + }) + + // Scenario: multiple saves for the same file are queued. + describe('Queueing', () => { + it('should serialize save requests for the same file', async () => { + // Arrange + const appId = 'app-1' + const fileId = 'file-1' + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftContent(fileId, 'draft-1') + let resolveFirst: (() => void) | undefined + mockMutateAsync.mockImplementationOnce(() => new Promise((resolve) => { + resolveFirst = resolve + })) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const first = result.current.saveFile(fileId) + await waitFor(() => { + expect(mockMutateAsync).toHaveBeenCalledTimes(1) + }) + store.getState().setDraftContent(fileId, 'draft-2') + const second = result.current.saveFile(fileId) + + // Assert + resolveFirst?.() + await first + await waitFor(() => { + expect(mockMutateAsync).toHaveBeenCalledTimes(2) + }) + await second + }) + }) + + // Scenario: saveAllDirty saves all relevant dirty files once. + describe('saveAllDirty', () => { + it('should save all dirty files except the start tab', async () => { + // Arrange + const appId = 'app-1' + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftContent('file-1', 'draft-1') + store.getState().setDraftMetadata('file-2', { tag: 'meta' }) + store.getState().setDraftContent(START_TAB_ID, 'start-draft') + setCachedContent(queryClient, appId, 'file-2', JSON.stringify({ content: 'meta-content' })) + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + result.current.saveAllDirty() + + // Assert + await waitFor(() => { + expect(mockMutateAsync).toHaveBeenCalledTimes(2) + }) + const nodeIds = mockMutateAsync.mock.calls.map(call => call[0].nodeId) + expect(nodeIds.sort()).toEqual(['file-1', 'file-2']) + }) + + it('should ignore dirty start tab when no other files are dirty', async () => { + // Arrange + const appId = 'app-1' + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftContent(START_TAB_ID, 'start-draft') + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + result.current.saveAllDirty() + + // Assert + await waitFor(() => { + expect(mockMutateAsync).not.toHaveBeenCalled() + }) + }) + }) +}) 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 8e93375c56..d7eb433f0a 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 @@ -30,9 +30,16 @@ export type SaveResult = { error?: unknown } +export type FallbackEntry = { + content: string + metadata?: Record +} + type SkillSaveContextValue = { saveFile: (fileId: string, options?: SaveFileOptions) => Promise saveAllDirty: () => void + registerFallback: (fileId: string, entry: FallbackEntry) => void + unregisterFallback: (fileId: string) => void } type SkillSaveProviderProps = { @@ -50,6 +57,7 @@ export const SkillSaveProvider = ({ const queryClient = useQueryClient() const updateContent = useUpdateAppAssetFileContent() const queueRef = useRef>>(new Map()) + const fallbackRegistryRef = useRef>(new Map()) const getCachedContent = useCallback((fileId: string): string | undefined => { if (!appId) @@ -89,8 +97,9 @@ export const SkillSaveProvider = ({ if (draftContent === undefined && !isMetadataDirty) return null - const metadata = state.fileMetadata.get(fileId) ?? fallbackMetadata - const content = draftContent ?? getCachedContent(fileId) ?? fallbackContent + const registryEntry = fallbackRegistryRef.current.get(fileId) + const metadata = state.fileMetadata.get(fileId) ?? fallbackMetadata ?? registryEntry?.metadata + const content = draftContent ?? getCachedContent(fileId) ?? fallbackContent ?? registryEntry?.content if (content === undefined) return null @@ -208,10 +217,20 @@ export const SkillSaveProvider = ({ void Promise.allSettled(tasks) }, [appId, saveFile, storeApi]) + const registerFallback = useCallback((fileId: string, entry: FallbackEntry) => { + fallbackRegistryRef.current.set(fileId, entry) + }, []) + + const unregisterFallback = useCallback((fileId: string) => { + fallbackRegistryRef.current.delete(fileId) + }, []) + const value = useMemo(() => ({ saveFile, saveAllDirty, - }), [saveAllDirty, saveFile]) + registerFallback, + unregisterFallback, + }), [saveAllDirty, saveFile, registerFallback, unregisterFallback]) return ( diff --git a/web/app/components/workflow/skill/main.tsx b/web/app/components/workflow/skill/main.tsx index 4faf8b04a5..20644a23e6 100644 --- a/web/app/components/workflow/skill/main.tsx +++ b/web/app/components/workflow/skill/main.tsx @@ -3,7 +3,6 @@ import type { FC } from 'react' import * as React from 'react' import { useStore as useAppStore } from '@/app/components/app/store' -import { useStore } from '@/app/components/workflow/store' import ContentArea from './content-area' import ContentBody from './content-body' import FileContentPanel from './file-content-panel' @@ -16,10 +15,7 @@ import SidebarSearchAdd from './sidebar-search-add' import SkillPageLayout from './skill-page-layout' const SkillAutoSaveManager: FC = () => { - const activeTabId = useStore(s => s.activeTabId) - - useSkillAutoSave({ activeTabId }) - + useSkillAutoSave() return null }