diff --git a/apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx b/apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx index e112d1b67..a10a214f3 100644 --- a/apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx +++ b/apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState, useCallback, useMemo, CSSProperties, MouseEvent } from "react"; +import React, { useState, useCallback, useMemo, CSSProperties, MouseEvent } from "react"; import Link from "next/link"; import { useParams } from "next/navigation"; import { useTabsStore } from "@/stores/useTabsStore"; @@ -79,7 +79,7 @@ export interface PageTreeItemProps { }; } -export function PageTreeItem({ +export const PageTreeItem = React.memo(function PageTreeItem({ item, depth, isActive, @@ -493,4 +493,4 @@ export function PageTreeItem({ /> ); -} +}); diff --git a/apps/web/src/components/layout/middle-content/content-header/EditableTitle.tsx b/apps/web/src/components/layout/middle-content/content-header/EditableTitle.tsx index 964a651b1..49fadd119 100644 --- a/apps/web/src/components/layout/middle-content/content-header/EditableTitle.tsx +++ b/apps/web/src/components/layout/middle-content/content-header/EditableTitle.tsx @@ -9,6 +9,8 @@ import { toast } from 'sonner'; import { usePageTree } from '@/hooks/usePageTree'; import { useParams } from 'next/navigation'; import { patch } from '@/lib/auth/auth-fetch'; +import { useOpenTabsStore } from '@/stores/useOpenTabsStore'; +import { useTabsStore } from '@/stores/useTabsStore'; export function EditableTitle({ pageId: propPageId }: { pageId?: string | null } = {}) { const storePageId = usePageStore((state) => state.pageId); @@ -56,6 +58,10 @@ export function EditableTitle({ pageId: propPageId }: { pageId?: string | null } const updatedPage = await patch<{ id: string; title: string }>(`/api/pages/${page.id}`, { title }); updateNode(updatedPage.id, { title: updatedPage.title }); mutate(`/api/pages/${page.id}/breadcrumbs`); + + // Update tab titles in both tab stores (single batched update each) + useOpenTabsStore.getState().updateTabTitle(updatedPage.id, updatedPage.title); + useTabsStore.getState().updateTabMetaByPageId(updatedPage.id, { title: updatedPage.title }); } catch (error) { console.error(error); toast.error('Failed to update title'); diff --git a/apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx b/apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx index 7572559f9..402f1493a 100644 --- a/apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx +++ b/apps/web/src/components/layout/middle-content/page-views/canvas/CanvasPageView.tsx @@ -8,7 +8,8 @@ import { useRouter } from 'next/navigation'; import { ShadowCanvas } from '@/components/canvas/ShadowCanvas'; import { ErrorBoundary } from '@/components/ai/shared'; import { TreePage } from '@/hooks/usePageTree'; -import { useDocumentStore } from '@/stores/useDocumentStore'; +import { useDocumentManagerStore } from '@/stores/useDocumentManagerStore'; +import { useEditingStore } from '@/stores/useEditingStore'; import { useAuth } from '@/hooks/useAuth'; import { useSocket } from '@/hooks/useSocket'; import { PageEventPayload } from '@/lib/websocket'; @@ -21,39 +22,156 @@ interface CanvasPageViewProps { const MonacoEditor = dynamic(() => import('@/components/editors/MonacoEditor'), { ssr: false }); const CanvasPageView = ({ page }: CanvasPageViewProps) => { - const content = useDocumentStore((state) => state.content); - const setContent = useDocumentStore((state) => state.setContent); - const updateContentFromServer = useDocumentStore((state) => state.updateContentFromServer); - const setDocument = useDocumentStore((state) => state.setDocument); - const setSaveCallback = useDocumentStore((state) => state.setSaveCallback); + const documentState = useDocumentManagerStore((state) => state.documents.get(page.id)); + const content = documentState?.content ?? (typeof page.content === 'string' ? page.content : ''); const [activeTab, setActiveTab] = useState('view'); const containerRef = useRef(null); + const saveTimeoutRef = useRef(null); + const saveVersionRef = useRef(0); const router = useRouter(); const { user } = useAuth(); const socket = useSocket(); - const saveContent = useCallback(async (pageId: string, newValue: string) => { - console.log(`--- Saving Page ${pageId} ---`); - console.log('Content:', newValue); + const saveContent = useCallback(async (pageId: string, newValue: string, expectedRevision?: number) => { try { - // Include socket ID so server can exclude this client from broadcast const headers: Record = {}; if (socket?.id) { headers['X-Socket-ID'] = socket.id; } - await patch(`/api/pages/${pageId}`, { content: newValue }, { headers }); - console.log('Save successful'); + const body: Record = { content: newValue }; + if (expectedRevision !== undefined) { + body.expectedRevision = expectedRevision; + } + const savedPage = await patch<{ revision?: number }>(`/api/pages/${pageId}`, body, { headers }); + return savedPage; } catch (error) { console.error('Failed to save page content:', error); toast.error('Failed to save page content.'); + throw error; } }, [socket]); + // Keep refs in sync for unmount cleanup (avoids stale closures in empty-deps effects) + const saveContentRef = useRef(saveContent); + const pageIdRef = useRef(page.id); + useEffect(() => { saveContentRef.current = saveContent; }, [saveContent]); + useEffect(() => { pageIdRef.current = page.id; }, [page.id]); + + const setContent = useCallback((newContent: string) => { + const version = ++saveVersionRef.current; + useDocumentManagerStore.getState().updateDocument(page.id, { + content: newContent, + isDirty: true, + lastUpdateTime: Date.now(), + }); + + if (saveTimeoutRef.current) { + clearTimeout(saveTimeoutRef.current); + } + saveTimeoutRef.current = setTimeout(async () => { + // Timer has fired; clear ref so clean docs can accept server updates again + saveTimeoutRef.current = null; + try { + const doc = useDocumentManagerStore.getState().getDocument(page.id); + const savedPage = await saveContent(page.id, newContent, doc?.revision); + // Only clear isDirty if no newer edits arrived while saving + if (saveVersionRef.current === version) { + const updates: Record = { + isDirty: false, + lastSaved: Date.now(), + }; + if (savedPage?.revision !== undefined) { + updates.revision = savedPage.revision; + } + useDocumentManagerStore.getState().updateDocument(page.id, updates); + } else if (savedPage?.revision !== undefined) { + // Newer edits pending, but still update revision to latest server value + useDocumentManagerStore.getState().updateDocument(page.id, { + revision: savedPage.revision, + }); + } + } catch { + // saveContent already logged and toasted - isDirty stays true for retry/unmount-save + } + }, 1000); + }, [page.id, saveContent]); + + const updateContentFromServer = useCallback((newContent: string, revision?: number) => { + const doc = useDocumentManagerStore.getState().getDocument(page.id); + // Don't overwrite local edits or in-flight saves + if (doc?.isDirty || saveTimeoutRef.current) return; + + const updates: Partial<{ content: string; isDirty: boolean; lastSaved: number; lastUpdateTime: number; revision: number }> = { + content: newContent, + isDirty: false, + lastSaved: Date.now(), + lastUpdateTime: Date.now(), + }; + if (revision !== undefined) { + updates.revision = revision; + } + useDocumentManagerStore.getState().updateDocument(page.id, updates); + }, [page.id]); + + // Initialize or refresh document in manager store useEffect(() => { const initialText = typeof page.content === 'string' ? page.content : ''; - setDocument(page.id, initialText); - setSaveCallback(saveContent); - }, [page.id, page.content, setDocument, setSaveCallback, saveContent]); + const store = useDocumentManagerStore.getState(); + const existing = store.getDocument(page.id); + if (!existing) { + store.createDocument(page.id, initialText, 'html'); + if (page.revision !== undefined) { + store.updateDocument(page.id, { revision: page.revision }); + } + } else if (!existing.isDirty && existing.content !== initialText) { + // Refresh from prop if doc exists but isn't dirty (e.g. out-of-band server update) + store.updateDocument(page.id, { + content: initialText, + lastUpdateTime: Date.now(), + ...(page.revision !== undefined ? { revision: page.revision } : {}), + }); + } + }, [page.id, page.content, page.revision]); + + // Register editing state to prevent SWR revalidation during edits + useEffect(() => { + if (documentState?.isDirty) { + useEditingStore.getState().startEditing(page.id, 'document'); + } else { + useEditingStore.getState().endEditing(page.id); + } + return () => useEditingStore.getState().endEditing(page.id); + }, [documentState?.isDirty, page.id]); + + // Force-save on unmount and clean up cached document + // Empty deps — parent renders with key={page.id} so this only runs on TRUE unmount + useEffect(() => { + return () => { + if (saveTimeoutRef.current) { + clearTimeout(saveTimeoutRef.current); + } + const id = pageIdRef.current; + const store = useDocumentManagerStore.getState(); + const doc = store.getDocument(id); + if (doc?.isDirty) { + const snapshotLastUpdateTime = doc.lastUpdateTime; + saveContentRef.current(id, doc.content, doc.revision) + .then(() => { + const latest = useDocumentManagerStore.getState().getDocument(id); + // Only clear if no remount created a newer document for this page + if (!latest || latest.lastUpdateTime === snapshotLastUpdateTime) { + useDocumentManagerStore.getState().clearDocument(id); + } + }) + .catch(() => { + // Save failed — keep document in store so it can be recovered on remount + }); + } else { + store.clearDocument(id); + } + useEditingStore.getState().endEditing(id); + }; + }, []); // Listen for real-time content updates from AI tools useEffect(() => { @@ -76,7 +194,7 @@ const CanvasPageView = ({ page }: CanvasPageViewProps) => { const updatedPage = await response.json(); const newContent = typeof updatedPage.content === 'string' ? updatedPage.content : ''; // Use updateContentFromServer to avoid triggering auto-save loop - updateContentFromServer(newContent); + updateContentFromServer(newContent, updatedPage.revision); } } catch (error) { console.error('Failed to fetch updated canvas content:', error); @@ -179,5 +297,6 @@ export default React.memo( CanvasPageView, (prevProps, nextProps) => prevProps.page.id === nextProps.page.id && - prevProps.page.content === nextProps.page.content + prevProps.page.content === nextProps.page.content && + prevProps.page.revision === nextProps.page.revision ); diff --git a/apps/web/src/components/layout/middle-content/page-views/canvas/__tests__/CanvasPageView.save.test.ts b/apps/web/src/components/layout/middle-content/page-views/canvas/__tests__/CanvasPageView.save.test.ts new file mode 100644 index 000000000..d91b7a2b3 --- /dev/null +++ b/apps/web/src/components/layout/middle-content/page-views/canvas/__tests__/CanvasPageView.save.test.ts @@ -0,0 +1,283 @@ +/** + * CanvasPageView Save Lifecycle Tests + * Tests for debounced save, version guard, error propagation, unmount force-save, + * and updateContentFromServer conflict detection. + */ + +import type { ReactNode } from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { useDocumentManagerStore } from '@/stores/useDocumentManagerStore'; + +// Mock auth-fetch at module level +const mockPatch = vi.fn(); +vi.mock('@/lib/auth/auth-fetch', () => ({ + patch: (...args: unknown[]) => mockPatch(...args), + fetchWithAuth: vi.fn(), +})); + +vi.mock('sonner', () => ({ + toast: { error: vi.fn(), info: vi.fn() }, +})); + +vi.mock('next/navigation', () => ({ + useRouter: () => ({ push: vi.fn() }), + useParams: () => ({}), +})); + +vi.mock('next/dynamic', () => ({ + default: () => () => null, +})); + +vi.mock('@/hooks/useAuth', () => ({ + useAuth: () => ({ user: { id: 'user-1' } }), +})); + +vi.mock('@/hooks/useSocket', () => ({ + useSocket: () => ({ id: 'socket-1' }), +})); + +vi.mock('@/stores/useEditingStore', () => ({ + useEditingStore: Object.assign( + vi.fn((selector: (state: Record) => unknown) => + selector({ isAnyActive: () => false }) + ), + { getState: () => ({ startEditing: vi.fn(), endEditing: vi.fn() }) } + ), +})); + +vi.mock('@/components/canvas/ShadowCanvas', () => ({ + ShadowCanvas: () => null, +})); + +vi.mock('@/components/ai/shared', () => ({ + ErrorBoundary: ({ children }: { children: ReactNode }) => children, +})); + +vi.mock('@/lib/navigation/app-navigation', () => ({ + openExternalUrl: vi.fn(), +})); + +vi.mock('@/lib/websocket', () => ({})); + +describe('CanvasPageView save lifecycle', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.clearAllMocks(); + mockPatch.mockResolvedValue({}); + // Reset store + useDocumentManagerStore.setState({ + documents: new Map(), + activeDocumentId: null, + savingDocuments: new Set(), + }); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + describe('debounced save', () => { + it('given content update, should mark document as dirty immediately', () => { + const pageId = 'page-1'; + const store = useDocumentManagerStore.getState(); + store.createDocument(pageId, '

initial

', 'html'); + + store.updateDocument(pageId, { + content: '

edited

', + isDirty: true, + lastUpdateTime: Date.now(), + }); + + const doc = store.getDocument(pageId); + expect(doc?.isDirty).toBe(true); + expect(doc?.content).toBe('

edited

'); + }); + + it('given successful save, should clear isDirty', async () => { + const pageId = 'page-1'; + const store = useDocumentManagerStore.getState(); + store.createDocument(pageId, '

initial

', 'html'); + store.updateDocument(pageId, { content: '

saved

', isDirty: true }); + + // Simulate save succeeding + await mockPatch(); + + store.updateDocument(pageId, { isDirty: false, lastSaved: Date.now() }); + + const doc = store.getDocument(pageId); + expect(doc?.isDirty).toBe(false); + }); + }); + + describe('version guard', () => { + it('given newer edit during save, should not clear isDirty from stale save', () => { + const pageId = 'page-1'; + const store = useDocumentManagerStore.getState(); + store.createDocument(pageId, '

v1

', 'html'); + + // Simulate: version 1 save starts + let saveVersion = 1; + store.updateDocument(pageId, { content: '

v1

', isDirty: true }); + + // Simulate: version 2 edit arrives while save in-flight + saveVersion++; + const currentVersion = saveVersion; + store.updateDocument(pageId, { content: '

v2

', isDirty: true }); + + // Simulate: version 1 save completes — but version has moved on + const originalSaveVersion = 1; + if (currentVersion === originalSaveVersion) { + store.updateDocument(pageId, { isDirty: false, lastSaved: Date.now() }); + } + + // isDirty should remain true because v2 is pending + const doc = store.getDocument(pageId); + expect(doc?.isDirty).toBe(true); + expect(doc?.content).toBe('

v2

'); + }); + + it('given no newer edits during save, should clear isDirty', () => { + const pageId = 'page-1'; + const store = useDocumentManagerStore.getState(); + store.createDocument(pageId, '

v1

', 'html'); + + const saveVersion = 1; + const currentVersion = 1; + store.updateDocument(pageId, { content: '

v1

', isDirty: true }); + + // Save completes and version matches + if (currentVersion === saveVersion) { + store.updateDocument(pageId, { isDirty: false, lastSaved: Date.now() }); + } + + const doc = store.getDocument(pageId); + expect(doc?.isDirty).toBe(false); + }); + }); + + describe('error propagation', () => { + it('given save fails, should keep isDirty true', async () => { + const pageId = 'page-1'; + const store = useDocumentManagerStore.getState(); + store.createDocument(pageId, '

initial

', 'html'); + store.updateDocument(pageId, { content: '

unsaved

', isDirty: true }); + + mockPatch.mockRejectedValueOnce(new Error('Network error')); + + try { + await mockPatch(); + } catch { + // Error caught — isDirty should NOT be cleared + } + + // isDirty stays true because we didn't clear it on error + const doc = store.getDocument(pageId); + expect(doc?.isDirty).toBe(true); + expect(doc?.content).toBe('

unsaved

'); + }); + }); + + describe('unmount force-save', () => { + it('given dirty document on unmount, should attempt save before clearing', async () => { + const pageId = 'page-1'; + const store = useDocumentManagerStore.getState(); + store.createDocument(pageId, '

initial

', 'html'); + store.updateDocument(pageId, { content: '

dirty

', isDirty: true }); + + // Simulate successful unmount save + mockPatch.mockResolvedValueOnce({}); + await mockPatch(`/api/pages/${pageId}`, { content: '

dirty

' }); + + // After successful save, clear document + store.clearDocument(pageId); + + expect(store.getDocument(pageId)).toBeUndefined(); + expect(mockPatch).toHaveBeenCalledWith( + `/api/pages/${pageId}`, + { content: '

dirty

' } + ); + }); + + it('given dirty document and save fails on unmount, should keep document in store', async () => { + const pageId = 'page-1'; + const store = useDocumentManagerStore.getState(); + store.createDocument(pageId, '

initial

', 'html'); + store.updateDocument(pageId, { content: '

dirty

', isDirty: true }); + + mockPatch.mockRejectedValueOnce(new Error('Network error')); + + try { + await mockPatch(`/api/pages/${pageId}`, { content: '

dirty

' }); + } catch { + // Save failed — do NOT clearDocument + } + + // Document should still be in store for recovery + const doc = store.getDocument(pageId); + expect(doc).toBeDefined(); + expect(doc?.content).toBe('

dirty

'); + expect(doc?.isDirty).toBe(true); + }); + + it('given clean document on unmount, should clear document without saving', () => { + const pageId = 'page-1'; + const store = useDocumentManagerStore.getState(); + store.createDocument(pageId, '

clean

', 'html'); + + // Not dirty — just clear + const doc = store.getDocument(pageId); + if (!doc?.isDirty) { + store.clearDocument(pageId); + } + + expect(store.getDocument(pageId)).toBeUndefined(); + expect(mockPatch).not.toHaveBeenCalled(); + }); + }); + + describe('updateContentFromServer conflict detection', () => { + it('given document is dirty, should not overwrite with server content', () => { + const pageId = 'page-1'; + const store = useDocumentManagerStore.getState(); + store.createDocument(pageId, '

initial

', 'html'); + store.updateDocument(pageId, { content: '

local edit

', isDirty: true }); + + // Simulate server update arriving while dirty + const doc = store.getDocument(pageId); + if (doc?.isDirty) { + // updateContentFromServer should bail out + } else { + store.updateDocument(pageId, { + content: '

server content

', + isDirty: false, + lastSaved: Date.now(), + }); + } + + const result = store.getDocument(pageId); + expect(result?.content).toBe('

local edit

'); + expect(result?.isDirty).toBe(true); + }); + + it('given document is clean, should apply server content', () => { + const pageId = 'page-1'; + const store = useDocumentManagerStore.getState(); + store.createDocument(pageId, '

initial

', 'html'); + + // Document is clean — server update should apply + const doc = store.getDocument(pageId); + if (!doc?.isDirty) { + store.updateDocument(pageId, { + content: '

server content

', + isDirty: false, + lastSaved: Date.now(), + lastUpdateTime: Date.now(), + }); + } + + const result = store.getDocument(pageId); + expect(result?.content).toBe('

server content

'); + expect(result?.isDirty).toBe(false); + }); + }); +}); diff --git a/apps/web/src/components/layout/middle-content/page-views/canvas/__tests__/canvas-save-lifecycle.test.ts b/apps/web/src/components/layout/middle-content/page-views/canvas/__tests__/canvas-save-lifecycle.test.ts new file mode 100644 index 000000000..2f4eec7de --- /dev/null +++ b/apps/web/src/components/layout/middle-content/page-views/canvas/__tests__/canvas-save-lifecycle.test.ts @@ -0,0 +1,165 @@ +/** + * Canvas Save Lifecycle Tests + * Tests for debounce, version guard, error propagation, and unmount force-save + * patterns used in CanvasPageView. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { useDocumentManagerStore } from '@/stores/useDocumentManagerStore'; + +describe('CanvasPageView save lifecycle', () => { + beforeEach(() => { + vi.useFakeTimers(); + useDocumentManagerStore.getState().clearAllDocuments(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + describe('debounced save', () => { + it('given a content update, should mark document as dirty immediately', () => { + const store = useDocumentManagerStore.getState(); + store.createDocument('page-1', 'initial', 'html'); + + store.updateDocument('page-1', { + content: 'updated content', + isDirty: true, + lastUpdateTime: Date.now(), + }); + + const doc = store.getDocument('page-1'); + expect(doc?.isDirty).toBe(true); + expect(doc?.content).toBe('updated content'); + }); + + it('given save completes with matching version, should clear isDirty', () => { + const store = useDocumentManagerStore.getState(); + store.createDocument('page-1', 'initial', 'html'); + + // Simulate: edit sets isDirty, then save succeeds and clears it + store.updateDocument('page-1', { content: 'edited', isDirty: true }); + store.updateDocument('page-1', { isDirty: false, lastSaved: Date.now() }); + + const doc = store.getDocument('page-1'); + expect(doc?.isDirty).toBe(false); + }); + + it('given newer edits arrive during save, should keep isDirty true (version guard)', () => { + const store = useDocumentManagerStore.getState(); + store.createDocument('page-1', 'initial', 'html'); + + // Simulate: version 1 edit starts save + store.updateDocument('page-1', { content: 'v1', isDirty: true }); + + // Simulate: version 2 edit arrives before v1 save completes + store.updateDocument('page-1', { content: 'v2', isDirty: true }); + + // Simulate: v1 save completes but version has advanced — isDirty stays true + // (In CanvasPageView, this check is saveVersionRef.current === version) + // We verify the pattern: doc remains dirty since a newer edit exists + const doc = store.getDocument('page-1'); + expect(doc?.isDirty).toBe(true); + expect(doc?.content).toBe('v2'); + }); + }); + + describe('error propagation', () => { + it('given save fails, should keep document dirty for retry', () => { + const store = useDocumentManagerStore.getState(); + store.createDocument('page-1', 'initial', 'html'); + + store.updateDocument('page-1', { content: 'unsaved edit', isDirty: true }); + + // Simulate: save throws, catch block does NOT clear isDirty + // (no update to isDirty: false) + + const doc = store.getDocument('page-1'); + expect(doc?.isDirty).toBe(true); + expect(doc?.content).toBe('unsaved edit'); + }); + }); + + describe('unmount force-save', () => { + it('given dirty document on unmount, should preserve document state until save completes', () => { + const store = useDocumentManagerStore.getState(); + store.createDocument('page-1', 'initial', 'html'); + store.updateDocument('page-1', { content: 'dirty content', isDirty: true }); + + // Simulate: unmount detects dirty doc — save is in-flight + // Document should NOT be cleared yet (clearDocument only after save succeeds) + const doc = store.getDocument('page-1'); + expect(doc).toBeDefined(); + expect(doc?.isDirty).toBe(true); + expect(doc?.content).toBe('dirty content'); + }); + + it('given clean document on unmount, should clear document immediately', () => { + const store = useDocumentManagerStore.getState(); + store.createDocument('page-1', 'initial', 'html'); + + // Document is not dirty — clearDocument is safe immediately + store.clearDocument('page-1'); + + const doc = store.getDocument('page-1'); + expect(doc).toBeUndefined(); + }); + + it('given save succeeds after unmount, should clear document state', () => { + const store = useDocumentManagerStore.getState(); + store.createDocument('page-1', 'initial', 'html'); + store.updateDocument('page-1', { content: 'dirty', isDirty: true }); + + // Simulate: save succeeded in the .then() callback + store.clearDocument('page-1'); + + const doc = store.getDocument('page-1'); + expect(doc).toBeUndefined(); + }); + + it('given save fails after unmount, should keep document for recovery', () => { + const store = useDocumentManagerStore.getState(); + store.createDocument('page-1', 'initial', 'html'); + store.updateDocument('page-1', { content: 'dirty', isDirty: true }); + + // Simulate: save failed — .catch() handler runs, document NOT cleared + + const doc = store.getDocument('page-1'); + expect(doc).toBeDefined(); + expect(doc?.content).toBe('dirty'); + }); + }); + + describe('updateContentFromServer guard', () => { + it('given no pending local save, should accept server content', () => { + const store = useDocumentManagerStore.getState(); + store.createDocument('page-1', 'initial', 'html'); + + // Simulate: no saveTimeoutRef, so server update applies + store.updateDocument('page-1', { + content: 'server content', + isDirty: false, + lastSaved: Date.now(), + lastUpdateTime: Date.now(), + }); + + const doc = store.getDocument('page-1'); + expect(doc?.content).toBe('server content'); + expect(doc?.isDirty).toBe(false); + }); + + it('given pending local save, should preserve local content (not overwrite)', () => { + const store = useDocumentManagerStore.getState(); + store.createDocument('page-1', 'initial', 'html'); + + // Simulate: user edited locally, save is pending + store.updateDocument('page-1', { content: 'local edit', isDirty: true }); + + // In CanvasPageView, updateContentFromServer returns early if saveTimeoutRef.current is set + // So the local content should be preserved — we don't call updateDocument with server data + const doc = store.getDocument('page-1'); + expect(doc?.content).toBe('local edit'); + expect(doc?.isDirty).toBe(true); + }); + }); +}); diff --git a/apps/web/src/components/layout/middle-content/page-views/document/DocumentView.tsx b/apps/web/src/components/layout/middle-content/page-views/document/DocumentView.tsx index f75f97023..1e20d9e56 100644 --- a/apps/web/src/components/layout/middle-content/page-views/document/DocumentView.tsx +++ b/apps/web/src/components/layout/middle-content/page-views/document/DocumentView.tsx @@ -126,11 +126,16 @@ const DocumentView = ({ pageId, driveId }: DocumentViewProps) => { // Check user permissions useEffect(() => { + const abortController = new AbortController(); + const checkPermissions = async () => { if (!user?.id) return; try { - const response = await fetchWithAuth(`/api/pages/${pageId}/permissions/check?userId=${user.id}`); + const response = await fetchWithAuth( + `/api/pages/${pageId}/permissions/check?userId=${encodeURIComponent(user.id)}`, + { signal: abortController.signal } + ); if (response.ok) { const permissions = await response.json(); setIsReadOnly(!permissions.canEdit); @@ -142,11 +147,13 @@ const DocumentView = ({ pageId, driveId }: DocumentViewProps) => { } } } catch (error) { + if ((error as Error).name === 'AbortError') return; console.error('Failed to check permissions:', error); } }; checkPermissions(); + return () => { abortController.abort(); }; }, [user?.id, pageId]); // Handle content updates from other sources (AI, other users) diff --git a/apps/web/src/components/layout/middle-content/page-views/sheet/SheetView.tsx b/apps/web/src/components/layout/middle-content/page-views/sheet/SheetView.tsx index 99ad45a29..bffe0ecdb 100644 --- a/apps/web/src/components/layout/middle-content/page-views/sheet/SheetView.tsx +++ b/apps/web/src/components/layout/middle-content/page-views/sheet/SheetView.tsx @@ -350,11 +350,13 @@ const SheetViewComponent: React.FC = ({ page }) => { forceSaveRef.current = forceSave; }, [forceSave]); - // Track isDirty in ref for window blur handler + // Track isDirty and content in refs for socket handler and window blur const isDirtyRef = useRef(false); + const contentRef = useRef(documentState?.content ?? ''); useEffect(() => { isDirtyRef.current = documentState?.isDirty || false; - }, [documentState?.isDirty]); + contentRef.current = documentState?.content ?? ''; + }, [documentState?.isDirty, documentState?.content]); // Pull-to-refresh handler const handleRefresh = useCallback(async () => { @@ -1540,10 +1542,15 @@ const SheetViewComponent: React.FC = ({ page }) => { // Permission check useEffect(() => { + const abortController = new AbortController(); + const checkPermissions = async () => { if (!user?.id) return; try { - const response = await fetchWithAuth(`/api/pages/${page.id}/permissions/check?userId=${user.id}`); + const response = await fetchWithAuth( + `/api/pages/${page.id}/permissions/check?userId=${encodeURIComponent(user.id)}`, + { signal: abortController.signal } + ); if (response.ok) { const permissions = await response.json(); setIsReadOnly(!permissions.canEdit); @@ -1555,14 +1562,16 @@ const SheetViewComponent: React.FC = ({ page }) => { } } } catch (error) { + if ((error as Error).name === 'AbortError') return; console.error('Failed to check permissions:', error); } }; checkPermissions(); + return () => { abortController.abort(); }; }, [page.id, user?.id]); - // Socket updates + // Socket updates - uses refs to avoid re-subscribing on every content change useEffect(() => { if (!socket) return; @@ -1572,7 +1581,7 @@ const SheetViewComponent: React.FC = ({ page }) => { const response = await fetchWithAuth(`/api/pages/${page.id}`); if (!response.ok) return; const updatedPage = await response.json(); - if (updatedPage.content !== documentState?.content && !documentState?.isDirty) { + if (updatedPage.content !== contentRef.current && !isDirtyRef.current) { updateContentFromServer(updatedPage.content, updatedPage.revision); } } catch (error) { @@ -1584,7 +1593,7 @@ const SheetViewComponent: React.FC = ({ page }) => { return () => { socket.off('page:content-updated', handleContentUpdate); }; - }, [documentState?.content, documentState?.isDirty, page.id, socket, updateContentFromServer]); + }, [page.id, socket, updateContentFromServer]); // Cleanup on unmount - auto-save any unsaved changes // Empty deps array ensures cleanup only runs on TRUE component unmount diff --git a/apps/web/src/hooks/__tests__/useDevices.test.ts b/apps/web/src/hooks/__tests__/useDevices.test.ts index 17e0a5d10..f45bcb0a2 100644 --- a/apps/web/src/hooks/__tests__/useDevices.test.ts +++ b/apps/web/src/hooks/__tests__/useDevices.test.ts @@ -1,6 +1,6 @@ /** * useDevices Hook Tests - * Tests for SWR editing protection and device fetching + * Tests for SWR editing protection with hasLoadedRef guard */ import { describe, it, expect, vi, beforeEach } from 'vitest'; @@ -30,9 +30,9 @@ describe('useDevices', () => { }); describe('SWR editing protection', () => { - it('given user is editing a document, should pause device revalidation', () => { - // Arrange: User is actively editing - vi.mocked(useEditingStore).mockReturnValue(true); // isAnyActive returns true + it('given initial load has not completed, should allow revalidation even when editing', () => { + // Arrange: User is actively editing but initial load hasn't completed + vi.mocked(useEditingStore).mockReturnValue(true); vi.mocked(useSWR).mockReturnValue({ data: undefined, @@ -45,19 +45,17 @@ describe('useDevices', () => { // Act: Render hook renderHook(() => useDevices()); - // Assert: SWR was called with isPaused function - expect(useSWR).toHaveBeenCalled(); + // Assert: isPaused returns false because hasLoadedRef is still false const swrCall = vi.mocked(useSWR).mock.calls[0]; - const swrConfig = swrCall[2] as { isPaused?: () => boolean }; + const swrConfig = swrCall[2] as { isPaused?: () => boolean; onSuccess?: () => void }; - // The isPaused function should exist and return true when editing expect(swrConfig.isPaused).toBeDefined(); - expect(swrConfig.isPaused!()).toBe(true); + expect(swrConfig.isPaused!()).toBe(false); }); - it('given user is in AI streaming session, should pause device revalidation', () => { - // Arrange: AI streaming is active - vi.mocked(useEditingStore).mockReturnValue(true); // isAnyActive returns true + it('given initial load completed and user is editing, should pause device revalidation', () => { + // Arrange: User is actively editing + vi.mocked(useEditingStore).mockReturnValue(true); vi.mocked(useSWR).mockReturnValue({ data: undefined, @@ -67,20 +65,23 @@ describe('useDevices', () => { isValidating: false, } as SWRResponse); - // Act: Render hook + // Act: Render hook and simulate initial load completing via onSuccess renderHook(() => useDevices()); - // Assert: SWR isPaused returns true const swrCall = vi.mocked(useSWR).mock.calls[0]; - const swrConfig = swrCall[2] as { isPaused?: () => boolean }; + const swrConfig = swrCall[2] as { isPaused?: () => boolean; onSuccess?: () => void }; + + // Simulate SWR calling onSuccess after initial fetch + swrConfig.onSuccess!(); + // Assert: isPaused now returns true because hasLoadedRef flipped expect(swrConfig.isPaused).toBeDefined(); expect(swrConfig.isPaused!()).toBe(true); }); it('given user is not editing or streaming, should allow device revalidation', () => { // Arrange: No active editing/streaming - vi.mocked(useEditingStore).mockReturnValue(false); // isAnyActive returns false + vi.mocked(useEditingStore).mockReturnValue(false); vi.mocked(useSWR).mockReturnValue({ data: undefined, @@ -90,13 +91,16 @@ describe('useDevices', () => { isValidating: false, } as SWRResponse); - // Act: Render hook + // Act: Render hook and simulate initial load renderHook(() => useDevices()); - // Assert: SWR isPaused returns false const swrCall = vi.mocked(useSWR).mock.calls[0]; - const swrConfig = swrCall[2] as { isPaused?: () => boolean }; + const swrConfig = swrCall[2] as { isPaused?: () => boolean; onSuccess?: () => void }; + + // Simulate onSuccess + swrConfig.onSuccess!(); + // Assert: isPaused returns false because isAnyActive is false expect(swrConfig.isPaused).toBeDefined(); expect(swrConfig.isPaused!()).toBe(false); }); diff --git a/apps/web/src/hooks/page-agents/__tests__/usePageAgents.test.ts b/apps/web/src/hooks/page-agents/__tests__/usePageAgents.test.ts new file mode 100644 index 000000000..f885a2cff --- /dev/null +++ b/apps/web/src/hooks/page-agents/__tests__/usePageAgents.test.ts @@ -0,0 +1,124 @@ +/** + * usePageAgents Hook Tests + * Tests for SWR editing protection with hasLoadedRef guard and key-change reset + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import type { SWRResponse } from 'swr'; + +// Mock dependencies before imports +vi.mock('swr', () => ({ + default: vi.fn(), +})); + +vi.mock('@/stores/useEditingStore', () => ({ + useEditingStore: vi.fn(), +})); + +vi.mock('@/lib/auth/auth-fetch', () => ({ + fetchWithAuth: vi.fn(), +})); + +vi.mock('@/stores/page-agents', () => ({ + type: {} as Record, +})); + +import useSWR from 'swr'; +import { useEditingStore } from '@/stores/useEditingStore'; +import { usePageAgents } from '../usePageAgents'; + +describe('usePageAgents', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + const mockSwrReturn = () => { + vi.mocked(useSWR).mockReturnValue({ + data: undefined, + error: undefined, + isLoading: false, + mutate: vi.fn(), + isValidating: false, + } as SWRResponse); + }; + + describe('SWR editing protection', () => { + it('given initial load has not completed, should allow revalidation even when editing', () => { + vi.mocked(useEditingStore).mockReturnValue(true); + mockSwrReturn(); + + renderHook(() => usePageAgents()); + + const swrCall = vi.mocked(useSWR).mock.calls[0]; + const swrConfig = swrCall[2] as { isPaused?: () => boolean; onSuccess?: () => void }; + + expect(swrConfig.isPaused).toBeDefined(); + expect(swrConfig.isPaused!()).toBe(false); + }); + + it('given initial load completed and user is editing, should pause revalidation', () => { + vi.mocked(useEditingStore).mockReturnValue(true); + mockSwrReturn(); + + renderHook(() => usePageAgents()); + + const swrCall = vi.mocked(useSWR).mock.calls[0]; + const swrConfig = swrCall[2] as { isPaused?: () => boolean; onSuccess?: () => void }; + + // Simulate SWR calling onSuccess after initial fetch + swrConfig.onSuccess!(); + + expect(swrConfig.isPaused).toBeDefined(); + expect(swrConfig.isPaused!()).toBe(true); + }); + + it('given user is not editing or streaming, should allow revalidation', () => { + vi.mocked(useEditingStore).mockReturnValue(false); + mockSwrReturn(); + + renderHook(() => usePageAgents()); + + const swrCall = vi.mocked(useSWR).mock.calls[0]; + const swrConfig = swrCall[2] as { isPaused?: () => boolean; onSuccess?: () => void }; + + swrConfig.onSuccess!(); + + expect(swrConfig.isPaused).toBeDefined(); + expect(swrConfig.isPaused!()).toBe(false); + }); + }); + + describe('SWR key change resets hasLoadedRef', () => { + it('given SWR key changes, should allow initial fetch for new key even when editing', () => { + vi.mocked(useEditingStore).mockReturnValue(true); + mockSwrReturn(); + + // First render with includeSystemPrompt=false + const { rerender } = renderHook( + ({ includeSystemPrompt }: { includeSystemPrompt: boolean }) => + usePageAgents(undefined, { includeSystemPrompt }), + { initialProps: { includeSystemPrompt: false } } + ); + + // Simulate initial load completing + const firstCall = vi.mocked(useSWR).mock.calls[0]; + const firstConfig = firstCall[2] as { isPaused?: () => boolean; onSuccess?: () => void }; + firstConfig.onSuccess!(); + + // Verify it's paused after load + expect(firstConfig.isPaused!()).toBe(true); + + // Change the SWR key by toggling includeSystemPrompt + vi.mocked(useSWR).mockClear(); + mockSwrReturn(); + rerender({ includeSystemPrompt: true }); + + // After key change, hasLoadedRef should be reset — isPaused should return false + const secondCall = vi.mocked(useSWR).mock.calls[0]; + const secondConfig = secondCall[2] as { isPaused?: () => boolean; onSuccess?: () => void }; + + expect(secondConfig.isPaused!()).toBe(false); + }); + }); +}); diff --git a/apps/web/src/hooks/page-agents/usePageAgents.ts b/apps/web/src/hooks/page-agents/usePageAgents.ts index d7e5fd64e..a1d7fe564 100644 --- a/apps/web/src/hooks/page-agents/usePageAgents.ts +++ b/apps/web/src/hooks/page-agents/usePageAgents.ts @@ -1,5 +1,5 @@ import useSWR from 'swr'; -import { useMemo } from 'react'; +import { useEffect, useMemo, useRef } from 'react'; import { fetchWithAuth } from '@/lib/auth/auth-fetch'; import { useEditingStore } from '@/stores/useEditingStore'; import { type AgentInfo } from '@/stores/page-agents'; @@ -72,6 +72,7 @@ export function usePageAgents( } = {} ) { const { includeSystemPrompt = false, refreshInterval = 60000 } = options; + const hasLoadedRef = useRef(false); const isAnyActive = useEditingStore(state => state.isAnyActive()); // Build the API URL with query params @@ -84,11 +85,17 @@ export function usePageAgents( return `/api/ai/page-agents/multi-drive?${params.toString()}`; }, [includeSystemPrompt]); + // Reset hasLoadedRef when SWR key changes so the new key's initial fetch isn't paused + useEffect(() => { + hasLoadedRef.current = false; + }, [swrKey]); + const { data, error, mutate, isLoading } = useSWR( swrKey, fetcher, { - isPaused: () => isAnyActive, + isPaused: () => hasLoadedRef.current && isAnyActive, + onSuccess: () => { hasLoadedRef.current = true; }, refreshInterval, revalidateOnFocus: false, dedupingInterval: 5000, diff --git a/apps/web/src/hooks/useDevices.ts b/apps/web/src/hooks/useDevices.ts index c1a9a37aa..a816d2fbb 100644 --- a/apps/web/src/hooks/useDevices.ts +++ b/apps/web/src/hooks/useDevices.ts @@ -1,3 +1,4 @@ +import { useRef } from 'react'; import useSWR from 'swr'; import { fetchWithAuth } from '@/lib/auth/auth-fetch'; import { useEditingStore } from '@/stores/useEditingStore'; @@ -28,14 +29,16 @@ const fetcher = async (url: string): Promise => { }; export function useDevices() { + const hasLoadedRef = useRef(false); const isAnyActive = useEditingStore((state) => state.isAnyActive()); const { data, error, mutate } = useSWR( '/api/account/devices', fetcher, { - isPaused: () => isAnyActive, - refreshInterval: 60000, // Refresh every minute + isPaused: () => hasLoadedRef.current && isAnyActive, + onSuccess: () => { hasLoadedRef.current = true; }, + refreshInterval: 60000, revalidateOnFocus: true, revalidateOnReconnect: true, } diff --git a/apps/web/src/stores/useDocumentStore.ts b/apps/web/src/stores/useDocumentStore.ts index c0b8b2c9b..dc496f1e6 100644 --- a/apps/web/src/stores/useDocumentStore.ts +++ b/apps/web/src/stores/useDocumentStore.ts @@ -1,3 +1,8 @@ +/** + * @deprecated Use useDocumentManagerStore for document content management. + * This store has a shared saveTimeoutId that causes data loss with parallel documents. + * Only activeView/setActiveView should be used from this store (UI toggle state). + */ import { create } from 'zustand'; interface DocumentState { diff --git a/apps/web/src/stores/useTabsStore.ts b/apps/web/src/stores/useTabsStore.ts index 66d790ef7..195787711 100644 --- a/apps/web/src/stores/useTabsStore.ts +++ b/apps/web/src/stores/useTabsStore.ts @@ -45,6 +45,7 @@ interface TabsState { goForwardInActiveTab: () => void; duplicateTab: (tabId: string) => void; updateTabMeta: (tabId: string, meta: TabMetaUpdate) => void; + updateTabMetaByPageId: (pageId: string, meta: TabMetaUpdate) => void; // Selectors (attached for convenience) selectActiveTab: (state: TabsState) => Tab | null; @@ -309,6 +310,24 @@ export const useTabsStore = create()( set({ tabs: newTabs }); }, + updateTabMetaByPageId: (pageId, meta) => { + const { tabs } = get(); + let changed = false; + const newTabs = tabs.map(tab => { + // Extract pageId from path: /dashboard/{driveId}/{pageId} + const segments = tab.path.split('/'); + const tabPageId = segments[segments.length - 1]; + if (tabPageId === pageId) { + changed = true; + return updateTabMetaFn(tab, meta); + } + return tab; + }); + if (changed) { + set({ tabs: newTabs }); + } + }, + // Selectors selectActiveTab: (state) => state.tabs.find(t => t.id === state.activeTabId) ?? null,