From e6e9b7ae940807c1578ba5fdf1c5ea8492971b92 Mon Sep 17 00:00:00 2001 From: hjlarry Date: Fri, 27 Mar 2026 17:30:21 +0800 Subject: [PATCH] fix(skill): fallback to direct save for collaboration followers on sync failure or timeout --- .../skills/skill-collaboration-manager.ts | 10 +- .../hooks/use-skill-save-manager.spec.tsx | 115 +++++++++++++++++- .../skill/hooks/use-skill-save-manager.tsx | 65 +++++++++- 3 files changed, 181 insertions(+), 9 deletions(-) diff --git a/web/app/components/workflow/collaboration/skills/skill-collaboration-manager.ts b/web/app/components/workflow/collaboration/skills/skill-collaboration-manager.ts index 6cd1a2476c..2690c96d96 100644 --- a/web/app/components/workflow/collaboration/skills/skill-collaboration-manager.ts +++ b/web/app/components/workflow/collaboration/skills/skill-collaboration-manager.ts @@ -300,8 +300,8 @@ class SkillCollaborationManager { return this.docs.has(fileId) } - requestSync(fileId: string): void { - this.emitSyncRequest(fileId) + requestSync(fileId: string): boolean { + return this.emitSyncRequest(fileId) } emitCursorUpdate(fileId: string, cursor: { start: number, end: number } | null): void { @@ -392,15 +392,17 @@ class SkillCollaborationManager { }) } - private emitSyncRequest(fileId: string): void { + private emitSyncRequest(fileId: string): boolean { if (!this.socket || !this.socket.connected) - return + return false emitWithAuthGuard(this.socket, 'collaboration_event', { type: 'skill_sync_request', data: { file_id: fileId }, timestamp: Date.now(), }) + + return true } private emitSkillFileActive(fileId: string, active: boolean): void { 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 9a81b4917a..adb1b7d9ec 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 @@ -3,15 +3,30 @@ 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 { useGlobalPublicStore } from '@/context/global-public-context' import { consoleQuery } from '@/service/client' import { START_TAB_ID } from '../constants' import { useSkillSaveManager } from './skill-save-context' import { SkillSaveProvider } from './use-skill-save-manager' -const { mockMutateAsync, mockToastSuccess, mockToastError } = vi.hoisted(() => ({ +const { + mockMutateAsync, + mockToastSuccess, + mockToastError, + mockIsFileCollaborative, + mockIsLeader, + mockRequestSync, + mockEmitFileSaved, + mockOnAnyFileSaved, +} = vi.hoisted(() => ({ mockMutateAsync: vi.fn(), mockToastSuccess: vi.fn(), mockToastError: vi.fn(), + mockIsFileCollaborative: vi.fn<(fileId: string) => boolean>(() => false), + mockIsLeader: vi.fn<(fileId: string) => boolean>(() => true), + mockRequestSync: vi.fn<(fileId: string) => boolean>(() => true), + mockEmitFileSaved: vi.fn<(fileId: string, content: string, metadata?: Record) => void>(), + mockOnAnyFileSaved: vi.fn<(callback: (payload: { file_id: string, content?: string, metadata?: Record }) => void) => () => void>(() => vi.fn()), })) vi.mock('@/service/use-app-asset', () => ({ @@ -27,6 +42,16 @@ vi.mock('@/app/components/base/ui/toast', () => ({ }, })) +vi.mock('../../collaboration/skills/skill-collaboration-manager', () => ({ + skillCollaborationManager: { + isFileCollaborative: (fileId: string) => mockIsFileCollaborative(fileId), + isLeader: (fileId: string) => mockIsLeader(fileId), + requestSync: (fileId: string) => mockRequestSync(fileId), + emitFileSaved: (fileId: string, content: string, metadata?: Record) => mockEmitFileSaved(fileId, content, metadata), + onAnyFileSaved: (callback: (payload: { file_id: string, content?: string, metadata?: Record }) => void) => mockOnAnyFileSaved(callback), + }, +})) + const createQueryClient = () => new QueryClient({ defaultOptions: { queries: { @@ -70,7 +95,20 @@ const getCachedPayload = (queryClient: QueryClient, appId: string, fileId: strin describe('useSkillSaveManager', () => { beforeEach(() => { vi.clearAllMocks() + vi.useRealTimers() mockMutateAsync.mockResolvedValue(undefined) + mockIsFileCollaborative.mockReturnValue(false) + mockIsLeader.mockReturnValue(true) + mockRequestSync.mockReturnValue(true) + mockOnAnyFileSaved.mockImplementation(() => vi.fn()) + + const currentFeatures = useGlobalPublicStore.getState().systemFeatures + useGlobalPublicStore.setState({ + systemFeatures: { + ...currentFeatures, + enable_collaboration_mode: false, + }, + }) }) it('should throw when used outside provider', () => { @@ -125,6 +163,81 @@ describe('useSkillSaveManager', () => { }) }) + // Scenario: follower saves should fall back to direct persistence if delegated sync cannot complete. + describe('Collaboration Fallback', () => { + it('should persist directly when sync request cannot be sent for a follower', async () => { + // Arrange + const appId = 'app-1' + const fileId = 'file-1' + const currentFeatures = useGlobalPublicStore.getState().systemFeatures + useGlobalPublicStore.setState({ + systemFeatures: { + ...currentFeatures, + enable_collaboration_mode: true, + }, + }) + mockIsFileCollaborative.mockReturnValue(true) + mockIsLeader.mockReturnValue(false) + mockRequestSync.mockReturnValue(false) + + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftContent(fileId, 'draft-content') + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const response = await result.current.saveFile(fileId) + + // Assert + expect(response.saved).toBe(true) + expect(mockRequestSync).toHaveBeenCalledWith(fileId) + expect(mockMutateAsync).toHaveBeenCalledWith({ + appId, + nodeId: fileId, + payload: { content: 'draft-content' }, + }) + }) + + it('should persist directly after delegated follower sync times out', async () => { + // Arrange + vi.useFakeTimers() + + const appId = 'app-1' + const fileId = 'file-1' + const currentFeatures = useGlobalPublicStore.getState().systemFeatures + useGlobalPublicStore.setState({ + systemFeatures: { + ...currentFeatures, + enable_collaboration_mode: true, + }, + }) + mockIsFileCollaborative.mockReturnValue(true) + mockIsLeader.mockReturnValue(false) + mockRequestSync.mockReturnValue(true) + + const store = createWorkflowStore({}) + const queryClient = createQueryClient() + const wrapper = createWrapper({ appId, store, queryClient }) + store.getState().setDraftContent(fileId, 'draft-content') + const { result } = renderHook(() => useSkillSaveManager(), { wrapper }) + + // Act + const saveTask = result.current.saveFile(fileId) + await vi.advanceTimersByTimeAsync(1600) + const response = await saveTask + + // Assert + expect(response.saved).toBe(true) + expect(mockRequestSync).toHaveBeenCalledWith(fileId) + expect(mockMutateAsync).toHaveBeenCalledWith({ + appId, + nodeId: fileId, + payload: { content: 'draft-content' }, + }) + }) + }) + // Scenario: successful saves update cache and clear draft state. describe('Saving', () => { it('should save draft content, update cache, and clear draft content', async () => { 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 cdf4efe3e4..fed9aa52cb 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 @@ -48,6 +48,13 @@ type SkillSaveProviderProps = { children: React.ReactNode } +type CollaborativeSaveWaiter = { + promise: Promise + cancel: () => void +} + +const COLLABORATION_SYNC_TIMEOUT_MS = 1500 + const normalizeMetadata = ( rawMetadata: Record | undefined, content: string, @@ -167,6 +174,44 @@ export const SkillSaveProvider = ({ patchFileContentCache(queryClient, queryKey, serialized) }, [appId, queryClient]) + const createCollaborativeSaveWaiter = useCallback((fileId: string): CollaborativeSaveWaiter => { + let settled = false + let unsubscribe: (() => void) | null = null + let timeoutId: ReturnType | null = null + + const promise = new Promise((resolve) => { + const finish = (saved: boolean) => { + if (settled) + return + settled = true + if (timeoutId !== null) + clearTimeout(timeoutId) + unsubscribe?.() + resolve(saved) + } + + unsubscribe = skillCollaborationManager.onAnyFileSaved((payload) => { + if (!payload || payload.file_id !== fileId) + return + finish(true) + }) + + timeoutId = setTimeout(() => finish(false), COLLABORATION_SYNC_TIMEOUT_MS) + }) + + return { + promise, + cancel: () => { + if (settled) + return + settled = true + if (timeoutId !== null) + clearTimeout(timeoutId) + unsubscribe?.() + }, + } + }, []) + const performSave = useCallback(async ( fileId: string, options?: SaveFileOptions, @@ -174,9 +219,21 @@ export const SkillSaveProvider = ({ if (!appId || !fileId || fileId === START_TAB_ID) return { saved: false } - if (isCollaborationEnabled && skillCollaborationManager.isFileCollaborative(fileId) && !skillCollaborationManager.isLeader(fileId)) { - skillCollaborationManager.requestSync(fileId) - return { saved: false } + const isCollaborativeFollower = isCollaborationEnabled + && skillCollaborationManager.isFileCollaborative(fileId) + && !skillCollaborationManager.isLeader(fileId) + + if (isCollaborativeFollower) { + const delegatedSaveWaiter = createCollaborativeSaveWaiter(fileId) + const didRequestSync = skillCollaborationManager.requestSync(fileId) + if (didRequestSync) { + const wasSavedByLeader = await delegatedSaveWaiter.promise + if (wasSavedByLeader) + return { saved: true } + } + else { + delegatedSaveWaiter.cancel() + } } const snapshot = buildSnapshot(fileId, options?.fallbackContent, options?.fallbackMetadata) @@ -219,7 +276,7 @@ export const SkillSaveProvider = ({ catch (error) { return { saved: false, error } } - }, [appId, buildSnapshot, isCollaborationEnabled, storeApi, updateCachedContent, updateFileContent]) + }, [appId, buildSnapshot, createCollaborativeSaveWaiter, isCollaborationEnabled, storeApi, updateCachedContent, updateFileContent]) const saveFile = useCallback(async ( fileId: string,