diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cc81ec5..ae910425 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 toggle. Previously the create call defaulted to including sub-pages, silently exposing every child of a freshly shared page. (#216) +- **Offline reading support**: opened pages, their sidebar tree, breadcrumb + children, and comments are cached in IndexedDB (TanStack Query persister plus + `y-indexeddb` for the page's Yjs document), and a PWA service worker + (vite-plugin-pwa) serves an app shell so previously opened pages stay readable + offline. The offline cache (persisted query cache, Yjs page documents, and the + service-worker API cache) is cleared on logout AND on sign-in so a previous + user's private data does not remain in the browser. +- **Mobile bootstrap**: a `returnToken` opt-in on login so native/mobile clients + can request the access JWT in the response body (`data.authToken`) in addition + to the httpOnly cookie (the web client stays cookie-only); an optional + OpenAPI/Swagger UI at `/api/docs` gated by `SWAGGER_ENABLED` (off by default); + and new env vars `CORS_ALLOWED_ORIGINS`, `SWAGGER_ENABLED`, `CAP_SERVER_URL`. + +### Changed + +- **CORS is now an explicit allowlist** (replaces the previous unconfigured + `app.enableCors()`). The same-origin web client is unaffected, but any + separately-hosted cross-domain client must now be listed in + `CORS_ALLOWED_ORIGINS` (native Capacitor/Ionic/localhost WebView origins are + allowed automatically). Requests with no `Origin` header (server-to-server) + are still allowed. + ### Fixed - **Internal links in exported Markdown no longer lose their visible text.** A @@ -333,18 +355,6 @@ embeds — plus a large batch of security hardening and test coverage. injected into the `` of public share pages only (for analytics such as Google Analytics or Yandex.Metrika), kept separate from the member-facing HTML-embed feature. -- **Offline reading support**: opened pages, their sidebar tree, breadcrumb - children, and comments are cached in IndexedDB (TanStack Query persister plus - `y-indexeddb` for the page's Yjs document), and a PWA service worker - (vite-plugin-pwa) serves an app shell so previously opened pages stay readable - offline. The offline cache (persisted query cache, Yjs page documents, and the - service-worker API cache) is cleared on logout so a previous user's private - data does not remain in the browser. -- **Mobile bootstrap**: a `returnToken` opt-in on login so native/mobile clients - can request the access JWT in the response body (`data.authToken`) in addition - to the httpOnly cookie (the web client stays cookie-only); an optional - OpenAPI/Swagger UI at `/api/docs` gated by `SWAGGER_ENABLED` (off by default); - and new env vars `CORS_ALLOWED_ORIGINS`, `SWAGGER_ENABLED`, `CAP_SERVER_URL`. - **MCP**: a hierarchical tree mode for `list_pages`, and per-user auth for the embedded `/mcp` endpoint. - **Page tree**: Expand all / Collapse all for the space tree, and @@ -360,12 +370,6 @@ embeds — plus a large batch of security hardening and test coverage. ### Changed -- **CORS is now an explicit allowlist** (replaces the previous unconfigured - `app.enableCors()`). The same-origin web client is unaffected, but any - separately-hosted cross-domain client must now be listed in - `CORS_ALLOWED_ORIGINS` (native Capacitor/Ionic/localhost WebView origins are - allowed automatically). Requests with no `Origin` header (server-to-server) - are still allowed. - HTML embed blocks now render inside a sandboxed iframe (separate origin) and, when the workspace HTML-embed toggle is on, can be inserted by any member (previously admin-only). Turning the toggle off hides existing embeds and diff --git a/apps/client/public/locales/en-US/translation.json b/apps/client/public/locales/en-US/translation.json index 7b304301..da87e4c1 100644 --- a/apps/client/public/locales/en-US/translation.json +++ b/apps/client/public/locales/en-US/translation.json @@ -473,6 +473,9 @@ "Saving page for offline use...": "Saving page for offline use...", "Page is now available offline": "Page is now available offline", "Failed to make page available offline": "Failed to make page available offline", + "You're offline": "You're offline", + "This page hasn't been saved for offline use, so it can't be loaded right now. Reconnect to the internet and try again.": "This page hasn't been saved for offline use, so it can't be loaded right now. Reconnect to the internet and try again.", + "Retry": "Retry", "Table of contents": "Table of contents", "Add headings (H1, H2, H3) to generate a table of contents.": "Add headings (H1, H2, H3) to generate a table of contents.", "Share": "Share", diff --git a/apps/client/public/locales/ru-RU/translation.json b/apps/client/public/locales/ru-RU/translation.json index 6c9921e7..02b3f67b 100644 --- a/apps/client/public/locales/ru-RU/translation.json +++ b/apps/client/public/locales/ru-RU/translation.json @@ -483,6 +483,9 @@ "Saving page for offline use...": "Сохраняем страницу для офлайн-доступа…", "Page is now available offline": "Страница доступна офлайн", "Failed to make page available offline": "Не удалось сделать страницу доступной офлайн", + "You're offline": "Вы офлайн", + "This page hasn't been saved for offline use, so it can't be loaded right now. Reconnect to the internet and try again.": "Эта страница не была сохранена для офлайн-доступа, поэтому её нельзя загрузить сейчас. Подключитесь к интернету и попробуйте снова.", + "Retry": "Повторить", "Table of contents": "Оглавление", "Add headings (H1, H2, H3) to generate a table of contents.": "Добавьте заголовки (H1, H2, H3), чтобы создать оглавление.", "Share": "Поделиться", diff --git a/apps/client/src/features/auth/hooks/use-auth.test.ts b/apps/client/src/features/auth/hooks/use-auth.test.ts new file mode 100644 index 00000000..9c75ce85 --- /dev/null +++ b/apps/client/src/features/auth/hooks/use-auth.test.ts @@ -0,0 +1,91 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; + +// react-i18next: identity t() so the hook renders without an i18n provider. +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (k: string) => k }), +})); + +// react-router-dom: only useNavigate is used by the hook. +const navigateMock = vi.fn(); +vi.mock("react-router-dom", () => ({ + useNavigate: () => navigateMock, +})); + +// The auth service is the network boundary; stub login per test. +const loginMock = vi.fn(); +vi.mock("@/features/auth/services/auth-service", () => ({ + login: (...args: unknown[]) => loginMock(...args), + logout: vi.fn(), + forgotPassword: vi.fn(), + passwordReset: vi.fn(), + setupWorkspace: vi.fn(), + verifyUserToken: vi.fn(), +})); + +vi.mock("@/features/workspace/services/workspace-service.ts", () => ({ + acceptInvitation: vi.fn(), +})); + +// The offline cache purge is the unit under test — assert it is invoked. +const clearOfflineCacheMock = vi.fn(); +vi.mock("@/features/offline/clear-offline-cache", () => ({ + clearOfflineCache: () => clearOfflineCacheMock(), +})); + +// app-route helpers are pure config; provide deterministic values. +vi.mock("@/lib/app-route.ts", () => ({ + default: { AUTH: { LOGIN: "/login" }, HOME: "/home" }, + getPostLoginRedirect: () => "/home", +})); + +// Mantine notifications: avoid touching the DOM-bound notification system. +vi.mock("@mantine/notifications", () => ({ + notifications: { show: vi.fn() }, +})); + +import useAuth from "./use-auth"; + +beforeEach(() => { + navigateMock.mockReset(); + loginMock.mockReset(); + loginMock.mockResolvedValue(undefined); + clearOfflineCacheMock.mockReset(); + clearOfflineCacheMock.mockResolvedValue(undefined); +}); + +describe("useAuth.handleSignIn", () => { + it("clears the offline cache BEFORE logging in (cross-user leak guard)", async () => { + const order: string[] = []; + clearOfflineCacheMock.mockImplementation(async () => { + order.push("clear"); + }); + loginMock.mockImplementation(async () => { + order.push("login"); + }); + + const { result } = renderHook(() => useAuth()); + await act(async () => { + await result.current.signIn({ email: "b@x", password: "pw" } as any); + }); + + expect(clearOfflineCacheMock).toHaveBeenCalledTimes(1); + expect(loginMock).toHaveBeenCalledTimes(1); + // The purge must run before the new session's login resolves. + expect(order).toEqual(["clear", "login"]); + expect(navigateMock).toHaveBeenCalledWith("/home"); + }); + + it("does not block sign-in when the cache purge throws (best-effort)", async () => { + clearOfflineCacheMock.mockRejectedValue(new Error("idb unavailable")); + + const { result } = renderHook(() => useAuth()); + await act(async () => { + await result.current.signIn({ email: "b@x", password: "pw" } as any); + }); + + // Login still proceeds despite the cleanup failure. + expect(loginMock).toHaveBeenCalledTimes(1); + expect(navigateMock).toHaveBeenCalledWith("/home"); + }); +}); diff --git a/apps/client/src/features/auth/hooks/use-auth.ts b/apps/client/src/features/auth/hooks/use-auth.ts index 5b1a4453..fd835234 100644 --- a/apps/client/src/features/auth/hooks/use-auth.ts +++ b/apps/client/src/features/auth/hooks/use-auth.ts @@ -34,6 +34,20 @@ export default function useAuth() { const handleSignIn = async (data: ILogin) => { setIsLoading(true); + // Purge any previous user's offline data BEFORE signing in (mirrors logout). + // On a shared/kiosk device the prior session may have ended WITHOUT an + // explicit logout (cookie/JWT expiry, tab close, force-quit), leaving user + // A's persisted query cache (gitmost-rq-cache) and Yjs page bodies + // (page.) in IndexedDB. Without this purge user B would briefly read A's + // cached currentUser/pages/comments on first render (UserProvider serves the + // cached user) and A's page bodies would stay readable offline. Best-effort: + // never block sign-in on cache cleanup. + try { + await clearOfflineCache(); + } catch { + // best-effort: never block sign-in on cache cleanup + } + try { await login(data); setIsLoading(false); diff --git a/apps/client/src/features/editor/atoms/editor-atoms.ts b/apps/client/src/features/editor/atoms/editor-atoms.ts index 718118cc..31cae179 100644 --- a/apps/client/src/features/editor/atoms/editor-atoms.ts +++ b/apps/client/src/features/editor/atoms/editor-atoms.ts @@ -16,8 +16,6 @@ export const isLocalSyncedAtom = atom(false); // Remote (Hocuspocus) sync state for the current page's Y.Doc. export const isRemoteSyncedAtom = atom(false); -export const showAiMenuAtom = atom(false); - export const showLinkMenuAtom = atom(false); // Current page's edit mode — initialized from the user's saved preference on diff --git a/apps/client/src/features/editor/hooks/collab-token.test.ts b/apps/client/src/features/editor/hooks/collab-token.test.ts new file mode 100644 index 00000000..9d3ba9b0 --- /dev/null +++ b/apps/client/src/features/editor/hooks/collab-token.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// jwt-decode is mocked so we can drive the four token states deterministically +// (decode success with a chosen exp, or a thrown decode error). +const decodeMock = vi.hoisted(() => vi.fn()); +vi.mock("jwt-decode", () => ({ + jwtDecode: decodeMock, +})); + +import { collabTokenNeedsRefresh } from "./collab-token"; + +const NOW_MS = 1_000_000_000; // fixed "now" in ms (so NOW_MS/1000 seconds) + +beforeEach(() => { + decodeMock.mockReset(); +}); + +describe("collabTokenNeedsRefresh", () => { + it("returns true when there is no token (fetch a fresh one)", () => { + expect(collabTokenNeedsRefresh(undefined, NOW_MS)).toBe(true); + // jwtDecode must not even be called for a missing token. + expect(decodeMock).not.toHaveBeenCalled(); + }); + + it("returns true when the token is malformed (jwtDecode throws)", () => { + decodeMock.mockImplementation(() => { + throw new Error("invalid token"); + }); + expect(collabTokenNeedsRefresh("garbage", NOW_MS)).toBe(true); + }); + + it("returns false for a valid, not-yet-expired token (no reconnect)", () => { + // exp is in the future relative to NOW. + decodeMock.mockReturnValue({ exp: NOW_MS / 1000 + 60 }); + expect(collabTokenNeedsRefresh("good", NOW_MS)).toBe(false); + }); + + it("returns true for a valid but expired token (refresh + reconnect)", () => { + // exp is in the past relative to NOW. + decodeMock.mockReturnValue({ exp: NOW_MS / 1000 - 60 }); + expect(collabTokenNeedsRefresh("expired", NOW_MS)).toBe(true); + }); + + it("treats exp exactly equal to now as expired (>= boundary)", () => { + decodeMock.mockReturnValue({ exp: NOW_MS / 1000 }); + expect(collabTokenNeedsRefresh("boundary", NOW_MS)).toBe(true); + }); +}); diff --git a/apps/client/src/features/editor/hooks/collab-token.ts b/apps/client/src/features/editor/hooks/collab-token.ts new file mode 100644 index 00000000..2327cf7a --- /dev/null +++ b/apps/client/src/features/editor/hooks/collab-token.ts @@ -0,0 +1,26 @@ +import { jwtDecode } from "jwt-decode"; + +/** + * Decide whether a collab token must be refreshed before reconnecting after an + * onAuthenticationFailed event. Pure and side-effect free so the four token + * states can be unit-tested directly: + * - no token -> true (fetch a fresh one and reconnect) + * - undecodable/malformed -> true (jwtDecode throws -> refresh) + * - valid, not expired -> false (token is still good; do NOT reconnect) + * - valid, expired -> true (refresh + reconnect) + * + * `nowMs` is injectable for deterministic tests; it defaults to `Date.now()`. + */ +export function collabTokenNeedsRefresh( + token: string | undefined, + nowMs: number = Date.now(), +): boolean { + if (!token) return true; + try { + const payload = jwtDecode<{ exp: number }>(token); + return nowMs / 1000 >= payload.exp; + } catch { + // malformed/undecodable token -> refresh + return true; + } +} diff --git a/apps/client/src/features/editor/hooks/use-generate-page-title.test.tsx b/apps/client/src/features/editor/hooks/use-generate-page-title.test.tsx index 880611ae..fc999ff7 100644 --- a/apps/client/src/features/editor/hooks/use-generate-page-title.test.tsx +++ b/apps/client/src/features/editor/hooks/use-generate-page-title.test.tsx @@ -139,7 +139,7 @@ describe("useGeneratePageTitle", () => { ); }); - it("happy path: applies the title, refreshes cache, writes the field, broadcasts", async () => { + it("happy path: applies the title, refreshes cache, broadcasts, and does NOT write the editor", async () => { const store = createStore(); const titleEditor = makeTitleEditor(); store.set(pageEditorAtom as never, makePageEditor("pageA")); @@ -157,9 +157,11 @@ describe("useGeneratePageTitle", () => { title: "Generated Title", }); expect(updatePageDataMock).toHaveBeenCalledWith(PAGE_A); - expect(titleEditor.commands.setContent).toHaveBeenCalledWith( - "Generated Title", - ); + // The title editor is bound to the Yjs `title` fragment; the server REST + // update reseeds that fragment and the reseed reaches the bound editor on + // its own. Writing here too would double/garble the title, so the hook must + // NOT touch the editor (regression guard for the Yjs duplication trap). + expect(titleEditor.commands.setContent).not.toHaveBeenCalled(); expect(localEmitMock).toHaveBeenCalled(); expect(emitMock).toHaveBeenCalled(); expect(notificationsShowMock).toHaveBeenCalledWith( @@ -167,7 +169,7 @@ describe("useGeneratePageTitle", () => { ); }); - it("does NOT write the visible title field when the user navigated away during generation", async () => { + it("keeps the DB write keyed by the captured pageId and still broadcasts after navigation", async () => { const store = createStore(); const titleEditor = makeTitleEditor(); // persistent across navigation store.set(pageEditorAtom as never, makePageEditor("pageA")); @@ -203,55 +205,9 @@ describe("useGeneratePageTitle", () => { pageId: "pageA", title: "Generated Title", }); - // ...but we must NOT stamp page A's title into page B's visible field. + // ...the hook never writes the editor regardless of navigation... expect(titleEditor.commands.setContent).not.toHaveBeenCalled(); - // The change is still broadcast to other clients. - expect(emitMock).toHaveBeenCalled(); - }); - - it("does NOT write the visible title field when the title editor is focused", async () => { - const store = createStore(); - const titleEditor = makeTitleEditor(); - store.set(pageEditorAtom as never, makePageEditor("pageA")); - store.set(titleEditorAtom as never, titleEditor); - - // Resolve generation under our control so we can mark the live title editor - // as focused before the post-generation write runs. - let resolveTitle!: (t: string) => void; - generatePageTitleMock.mockReturnValue( - new Promise((res) => { - resolveTitle = res; - }), - ); - updateTitleMock.mockResolvedValue(PAGE_A); - const { result } = setup("pageA", store); - - let pending!: Promise; - act(() => { - pending = result.current.mutateAsync(); - }); - - // The user clicked into the title field while the model ran — overwriting it - // now would clobber what they are actively typing. - act(() => { - (titleEditor as { isFocused: boolean }).isFocused = true; - }); - - await act(async () => { - resolveTitle("Generated Title"); - await pending; - }); - - // The DB write still persists the value... - expect(updateTitleMock).toHaveBeenCalledWith({ - pageId: "pageA", - title: "Generated Title", - }); - expect(updatePageDataMock).toHaveBeenCalledWith(PAGE_A); - // ...but the visible field is left alone while it is focused. - expect(titleEditor.commands.setContent).not.toHaveBeenCalled(); - // The change is still broadcast to other clients. - expect(localEmitMock).toHaveBeenCalled(); + // ...and the change is still broadcast to other clients. expect(emitMock).toHaveBeenCalled(); }); diff --git a/apps/client/src/features/editor/hooks/use-generate-page-title.ts b/apps/client/src/features/editor/hooks/use-generate-page-title.ts index e8d9e0e2..23b8a260 100644 --- a/apps/client/src/features/editor/hooks/use-generate-page-title.ts +++ b/apps/client/src/features/editor/hooks/use-generate-page-title.ts @@ -1,13 +1,9 @@ -import { useRef } from "react"; import { useMutation } from "@tanstack/react-query"; import { useAtomValue } from "jotai"; import { notifications } from "@mantine/notifications"; import { useTranslation } from "react-i18next"; import { htmlToMarkdown } from "@docmost/editor-ext"; -import { - pageEditorAtom, - titleEditorAtom, -} from "@/features/editor/atoms/editor-atoms.ts"; +import { pageEditorAtom } from "@/features/editor/atoms/editor-atoms.ts"; import { updatePageData, useUpdateTitlePageMutation, @@ -33,18 +29,9 @@ const MAX_CONTENT_CHARS = 20000; export function useGeneratePageTitle(pageId: string) { const { t } = useTranslation(); const pageEditor = useAtomValue(pageEditorAtom); - const titleEditor = useAtomValue(titleEditorAtom); const { mutateAsync: updateTitle } = useUpdateTitlePageMutation(); const emit = useQueryEmit(); - // The page/title editors come from GLOBAL atoms that re-point when the user - // navigates to another page. The mutation below awaits the model for 1-3s, and - // its closure captures the editors from the render that started it. Keep a live - // reference so the post-generation write targets whatever page is on screen - // *now*, not the page the generation was started from. - const editorsRef = useRef({ pageEditor, titleEditor }); - editorsRef.current = { pageEditor, titleEditor }; - return useMutation({ mutationFn: async () => { if (!pageEditor || pageEditor.isDestroyed) return; @@ -70,33 +57,15 @@ export function useGeneratePageTitle(pageId: string) { const page = await updateTitle({ pageId, title }); // POST /pages/update updatePageData(page); // refresh the react-query cache - // Reflect the new title in the field immediately. The button lives in the - // byline, so the title editor is not focused — setContent is safe and stays - // undoable through its History extension (Ctrl/Cmd+Z reverts the change). - // - // Guard against navigation during generation: if the user switched pages - // while the model ran, the (persistent) title editor now shows ANOTHER - // page, so writing here would drop page A's title into page B's visible - // field. page-editor.tsx stamps the live page editor with its pageId - // (`editor.storage.pageId`), mirroring TitleEditor's `activePageId !== - // pageId` guard — bail the visible write unless that live editor still - // belongs to the page this title was generated for. The DB write above is - // already correct (keyed by the captured `pageId`), and the broadcast below - // still propagates page A's change to other clients. - const livePageEditor = editorsRef.current.pageEditor; - const liveTitleEditor = editorsRef.current.titleEditor; - // `storage.pageId` is stamped untyped in page-editor.tsx's onCreate. - const livePageId = (livePageEditor?.storage as { pageId?: string }) - ?.pageId; - const stillOnPage = livePageId === pageId; - if ( - stillOnPage && - liveTitleEditor && - !liveTitleEditor.isDestroyed && - !liveTitleEditor.isFocused - ) { - liveTitleEditor.commands.setContent(page.title); - } + // Do NOT write the title into the editor here. The title editor is bound to + // the Yjs `title` fragment and Yjs is the source of truth. The server REST + // /pages/update reseeds that fragment (writePageTitle → writeTitleFragment, + // a full clear+replace) and the reseed reaches the bound title editor on + // its own as a remote provider update. The old REST-era setContent here + // would race that reseed and double/garble the title (the "Yjs duplication + // trap"), so it is intentionally omitted. The DB write above is keyed by + // the captured `pageId`, so it stays correct even if the user navigated + // away during generation. // Broadcast to other clients, mirroring TitleEditor.saveTitle's event shape. const event: UpdateEvent = { diff --git a/apps/client/src/features/editor/hooks/use-page-collab-providers.ts b/apps/client/src/features/editor/hooks/use-page-collab-providers.ts index a3f150eb..6a610733 100644 --- a/apps/client/src/features/editor/hooks/use-page-collab-providers.ts +++ b/apps/client/src/features/editor/hooks/use-page-collab-providers.ts @@ -24,7 +24,7 @@ import { IPage } from "@/features/page/types/page.types.ts"; import { useParams } from "react-router-dom"; import { extractPageSlugId } from "@/lib"; import { FIVE_MINUTES } from "@/lib/constants.ts"; -import { jwtDecode } from "jwt-decode"; +import { collabTokenNeedsRefresh } from "@/features/editor/hooks/collab-token"; export interface PageCollabProviders { ydoc: Y.Doc | null; @@ -70,16 +70,6 @@ export function usePageCollabProviders(pageId: string): PageCollabProviders { } | null>(null); const [providersReady, setProvidersReady] = useState(false); - // Mirror the local/remote sync flags into shared atoms so the header - // indicator can read them. These atoms are the single source of truth; the - // wrappers keep the existing call sites valid while driving only the atoms. - const setLocalSynced = (value: boolean) => { - setIsLocalSyncedAtom(value); - }; - const setRemoteSynced = (value: boolean) => { - setIsRemoteSyncedAtom(value); - }; - useEffect(() => { if (!providersRef.current) { const documentName = `page.${pageId}`; @@ -89,13 +79,13 @@ export function usePageCollabProviders(pageId: string): PageCollabProviders { url: collaborationURL, }); const onLocalSyncedHandler = () => { - setLocalSynced(true); + setIsLocalSyncedAtom(true); }; const onStatusHandler = (event: onStatusParameters) => { setYjsConnectionStatus(event.status); }; const onSyncedHandler = (event: onSyncedParameters) => { - setRemoteSynced(event.state); + setIsRemoteSyncedAtom(event.state); }; const onStatelessHandler = ({ payload }: onStatelessParameters) => { try { @@ -121,17 +111,7 @@ export function usePageCollabProviders(pageId: string): PageCollabProviders { // a refetch. A missing/malformed token must NOT crash the handler — // jwtDecode(undefined) throws — so treat any decode failure as "needs // refresh" and proceed to refetch + reconnect instead of getting stuck. - const token = collabTokenRef.current; - let needsRefresh = true; // no/unparseable token -> fetch a fresh one and reconnect - if (token) { - try { - const payload = jwtDecode<{ exp: number }>(token); - needsRefresh = Date.now() / 1000 >= payload.exp; - } catch { - needsRefresh = true; // malformed token -> refresh - } - } - if (!needsRefresh) return; + if (!collabTokenNeedsRefresh(collabTokenRef.current)) return; refetchCollabToken().then((result) => { if (result.data?.token) { socket.disconnect(); @@ -166,8 +146,8 @@ export function usePageCollabProviders(pageId: string): PageCollabProviders { providersRef.current?.local.destroy(); providersRef.current = null; // Reset shared sync state on page change/unmount. - setLocalSynced(false); - setRemoteSynced(false); + setIsLocalSyncedAtom(false); + setIsRemoteSyncedAtom(false); }; }, [pageId]); diff --git a/apps/client/src/features/editor/title-collab.test.ts b/apps/client/src/features/editor/title-collab.test.ts new file mode 100644 index 00000000..c95c66a4 --- /dev/null +++ b/apps/client/src/features/editor/title-collab.test.ts @@ -0,0 +1,33 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// isChangeOrigin is mocked so we can simulate local vs remote/collab-origin +// transactions without constructing a real ProseMirror/Yjs transaction. +const isChangeOriginMock = vi.hoisted(() => vi.fn()); +vi.mock("@tiptap/extension-collaboration", () => ({ + isChangeOrigin: isChangeOriginMock, +})); + +import { shouldPropagateTitleChange } from "./title-collab"; + +beforeEach(() => { + isChangeOriginMock.mockReset(); +}); + +describe("shouldPropagateTitleChange", () => { + it("propagates a genuine local edit (isChangeOrigin false)", () => { + isChangeOriginMock.mockReturnValue(false); + expect(shouldPropagateTitleChange({ local: true })).toBe(true); + expect(isChangeOriginMock).toHaveBeenCalledWith({ local: true }); + }); + + it("skips a remote/collab-origin update (isChangeOrigin true)", () => { + isChangeOriginMock.mockReturnValue(true); + expect(shouldPropagateTitleChange({ remote: true })).toBe(false); + }); + + it("propagates when there is no transaction (treated as local)", () => { + expect(shouldPropagateTitleChange(undefined)).toBe(true); + // isChangeOrigin must not be called for a missing transaction. + expect(isChangeOriginMock).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/client/src/features/editor/title-collab.ts b/apps/client/src/features/editor/title-collab.ts new file mode 100644 index 00000000..ccf3af7d --- /dev/null +++ b/apps/client/src/features/editor/title-collab.ts @@ -0,0 +1,19 @@ +import { isChangeOrigin } from "@tiptap/extension-collaboration"; + +/** + * Whether a TitleEditor `onUpdate` should drive URL + tree propagation. + * + * Only genuine LOCAL edits propagate. Remote/collab-origin Yjs updates + * (detected via `isChangeOrigin`) are skipped so a remote title change is not + * re-broadcast back, which would create a feedback loop. A missing transaction + * is treated as a local edit (propagate). + * + * Extracted as a pure helper so the skip decision is unit-testable without + * mounting the full collaborative editor. + */ +export function shouldPropagateTitleChange(transaction: unknown): boolean { + return !( + transaction && + isChangeOrigin(transaction as Parameters[0]) + ); +} diff --git a/apps/client/src/features/editor/title-editor.test.tsx b/apps/client/src/features/editor/title-editor.test.tsx new file mode 100644 index 00000000..b08ff4dd --- /dev/null +++ b/apps/client/src/features/editor/title-editor.test.tsx @@ -0,0 +1,87 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen } from "@testing-library/react"; + +// Drive the fallback-vs-collaborative switch (titleReady = providersReady && +// !!ydoc) by controlling what the editor-providers context returns. +const editorProvidersValue: { ydoc: unknown; providersReady: boolean } = { + ydoc: null, + providersReady: false, +}; +vi.mock("@/features/editor/contexts/editor-providers-context", () => ({ + useEditorProviders: () => editorProvidersValue, +})); + +// Mock the tiptap React bindings so the test does not mount a real editor: +// useEditor returns a minimal stub and EditorContent renders a marker. +vi.mock("@tiptap/react", () => ({ + useEditor: () => ({ + isInitialized: true, + commands: { focus: vi.fn() }, + setEditable: vi.fn(), + getText: () => "", + }), + EditorContent: () =>
, +})); + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (k: string) => k }), +})); + +const navigateMock = vi.fn(); +vi.mock("react-router-dom", () => ({ + useNavigate: () => navigateMock, +})); + +vi.mock("@/features/websocket/use-query-emit.ts", () => ({ + useQueryEmit: () => vi.fn(), +})); + +// page-query transitively imports @/main.tsx; mock it to a pure stub. +vi.mock("@/features/page/queries/page-query", () => ({ + updatePageData: vi.fn(), +})); +vi.mock("@/main.tsx", () => ({ + queryClient: { getQueryData: vi.fn(), setQueryData: vi.fn() }, +})); + +import { TitleEditor } from "./title-editor"; + +const baseProps = { + pageId: "p1", + slugId: "slug-1", + title: "My Page Title", + spaceSlug: "space", + editable: true, +}; + +beforeEach(() => { + navigateMock.mockReset(); + editorProvidersValue.ydoc = null; + editorProvidersValue.providersReady = false; +}); + +describe("TitleEditor fallback vs collaborative switch", () => { + it("renders a static

with the title before the shared doc is ready", () => { + editorProvidersValue.ydoc = null; + editorProvidersValue.providersReady = false; + + render(); + + const heading = screen.getByRole("heading", { level: 1 }); + expect(heading.textContent).toBe("My Page Title"); + // The collaborative editor must NOT mount until the doc is ready. + expect(screen.queryByTestId("collab-editor")).toBeNull(); + }); + + it("renders the collaborative editor once the shared doc is ready", () => { + editorProvidersValue.ydoc = {}; // truthy shared doc + editorProvidersValue.providersReady = true; + + render(); + + expect(screen.getByTestId("collab-editor")).toBeDefined(); + // The static fallback

is gone — Yjs is the single source of truth and + // the prop is never seeded into the collaborative editor. + expect(screen.queryByRole("heading", { level: 1 })).toBeNull(); + }); +}); diff --git a/apps/client/src/features/editor/title-editor.tsx b/apps/client/src/features/editor/title-editor.tsx index 11a457ea..7d1e3020 100644 --- a/apps/client/src/features/editor/title-editor.tsx +++ b/apps/client/src/features/editor/title-editor.tsx @@ -15,10 +15,8 @@ import { updatePageData } from "@/features/page/queries/page-query"; import { useDebouncedCallback, getHotkeyHandler } from "@mantine/hooks"; import { useAtom } from "jotai"; import { useQueryEmit } from "@/features/websocket/use-query-emit.ts"; -import { - Collaboration, - isChangeOrigin, -} from "@tiptap/extension-collaboration"; +import { Collaboration } from "@tiptap/extension-collaboration"; +import { shouldPropagateTitleChange } from "@/features/editor/title-collab"; import { buildPageUrl } from "@/features/page/page.utils.ts"; import { useNavigate } from "react-router-dom"; import { useTranslation } from "react-i18next"; @@ -99,7 +97,7 @@ export function TitleEditor({ onUpdate({ editor, transaction }) { // Drive URL + tree propagation only on genuine local edits; skip // remote/collab-origin Yjs updates to avoid feedback loops. - if (transaction && isChangeOrigin(transaction)) return; + if (!shouldPropagateTitleChange(transaction)) return; debouncedPropagateTitle(editor.getText()); }, editable: editable, diff --git a/apps/client/src/features/offline/make-offline.test.ts b/apps/client/src/features/offline/make-offline.test.ts index f54019ba..8673d6ac 100644 --- a/apps/client/src/features/offline/make-offline.test.ts +++ b/apps/client/src/features/offline/make-offline.test.ts @@ -123,17 +123,25 @@ describe("warmInfiniteAll", () => { expect(payload.pageParams).toEqual([undefined, "c1", "c2"]); }); - it("caps pagination at maxPages", async () => { + it("caps pagination at maxPages and reports the truncation (returns false)", async () => { // Always returns a non-null cursor — the cap is the only thing that stops it. const fetchPage = vi .fn() .mockResolvedValue({ items: [], meta: { nextCursor: "more" } }); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - await warmInfiniteAll(["comments", "p1"], fetchPage, 2); + // Hitting maxPages with a cursor still pending is a truncated warm: the + // (partial) cache is still written, but the result is reported as false. + await expect( + warmInfiniteAll(["comments", "p1"], fetchPage, 2), + ).resolves.toBe(false); expect(fetchPage).toHaveBeenCalledTimes(2); const payload = setQueryData.mock.calls[0][1]; expect(payload.pages).toHaveLength(2); + expect(errorSpy).toHaveBeenCalled(); + + errorSpy.mockRestore(); }); it("returns true on success", async () => { diff --git a/apps/client/src/features/offline/make-offline.ts b/apps/client/src/features/offline/make-offline.ts index e013f828..28b33bbb 100644 --- a/apps/client/src/features/offline/make-offline.ts +++ b/apps/client/src/features/offline/make-offline.ts @@ -16,6 +16,7 @@ import { spaceByIdQueryOptions } from "@/features/space/queries/space-query"; import { RQ_KEY } from "@/features/comment/queries/comment-query"; import { getPageComments } from "@/features/comment/services/comment-service"; import { getMyInfo } from "@/features/user/services/user-service"; +import { userKeys } from "@/features/user/hooks/use-current-user"; import { IPage } from "@/features/page/types/page.types"; import { IPagination } from "@/lib/types.ts"; @@ -32,7 +33,13 @@ import { IPagination } from "@/lib/types.ts"; * but it is reported — the error is logged with context and `false` is returned * so the caller can record the failed step instead of silently succeeding. * - * Returns true if the whole list was paginated and written, false on any error. + * Returns true ONLY if the cursor chain was fully exhausted and written. If the + * walk stops because it hit `maxPages` while a `nextCursor` is still pending, + * the cached list is truncated AND its last page keeps a nextCursor that cannot + * be re-fetched offline (hooks that gate on hasNextPage would spin forever), so + * that case is logged and returns false too — the caller records it as a failed + * warm instead of a silent truncated success. The (partial) cache is still + * written so what we did fetch is usable. * * Exported for unit testing of the cursor-walk / cache-write behavior. */ @@ -45,16 +52,31 @@ export async function warmInfiniteAll( const pages: IPagination[] = []; const pageParams: (string | undefined)[] = []; let cursor: string | undefined = undefined; + let exhausted = false; for (let i = 0; i < maxPages; i++) { const res = await fetchPage(cursor); pages.push(res); pageParams.push(cursor); cursor = res?.meta?.nextCursor ?? undefined; - if (!cursor) break; + if (!cursor) { + exhausted = true; + break; + } } queryClient.setQueryData(queryKey, { pages, pageParams }); + + if (!exhausted) { + // Stopped at maxPages with a cursor still pending: the list is truncated + // and the last cached page's nextCursor is un-fetchable offline. Report it + // as a failed warm rather than a silent truncated success. + console.error("warmInfiniteAll truncated at maxPages", { + queryKey, + maxPages, + }); + return false; + } return true; } catch (error) { console.error("warmInfiniteAll failed", { queryKey, error }); @@ -101,7 +123,7 @@ export async function makePageAvailableOffline({ // cache actually has an entry to restore. try { await queryClient.prefetchQuery({ - queryKey: ["currentUser"], + queryKey: userKeys.currentUser(), queryFn: () => getMyInfo(), }); } catch (error) { diff --git a/apps/client/src/features/offline/persist-roots.guard.test.ts b/apps/client/src/features/offline/persist-roots.guard.test.ts new file mode 100644 index 00000000..2487b673 --- /dev/null +++ b/apps/client/src/features/offline/persist-roots.guard.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from "vitest"; + +// The query modules transitively import the app entry (@/main.tsx) for the +// shared queryClient; mock it so importing the key factories has no side effects. +import { vi } from "vitest"; +vi.mock("@/main.tsx", () => ({ + queryClient: { setQueryData: vi.fn(), getQueryData: vi.fn() }, +})); + +import { OFFLINE_PERSIST_ROOTS } from "./query-persister"; +import { pageKeys } from "@/features/page/queries/page-query"; +import { spaceKeys } from "@/features/space/queries/space-query"; +import { RQ_KEY } from "@/features/comment/queries/comment-query"; +import { userKeys } from "@/features/user/hooks/use-current-user"; + +/** + * Architecture guard (#13): every string persisted via OFFLINE_PERSIST_ROOTS + * must be the ROOT (queryKey[0]) of some exported query-key factory. If a + * factory's root is renamed without updating the persist registry — or vice + * versa — offline persist/warm silently breaks (persisted keys never match the + * live queries). This turns that silent regression into a red build. + * + * Each factory is invoked with throwaway args; only queryKey[0] is inspected. + */ +function rootOf(key: readonly unknown[]): string { + return String(key[0]); +} + +const FACTORY_ROOTS = new Set([ + rootOf(pageKeys.detail("x")), + rootOf(pageKeys.sidebar({})), + rootOf(pageKeys.rootSidebar("x")), + rootOf(pageKeys.breadcrumbs("x")), + rootOf(pageKeys.recentChanges("x")), + rootOf(spaceKeys.detail("x")), + rootOf(spaceKeys.list()), + rootOf(RQ_KEY("x")), + rootOf(userKeys.currentUser()), +]); + +describe("OFFLINE_PERSIST_ROOTS is backed by real query-key factories", () => { + it("maps every persisted root to an exported factory root", () => { + const unbacked = [...OFFLINE_PERSIST_ROOTS].filter( + (root) => !FACTORY_ROOTS.has(root), + ); + expect(unbacked).toEqual([]); + }); +}); diff --git a/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx b/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx index 6bf42ed4..8df209fb 100644 --- a/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx +++ b/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx @@ -104,11 +104,16 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) { color: "red", }); } - } catch { + } catch (err) { // makePageAvailableOffline no longer throws, but warmPageYdoc and other - // unexpected failures stay guarded here. + // unexpected failures stay guarded here. Log the raw error and surface the + // real cause to the user instead of a bare generic string (AGENTS.md). + console.error("handleMakeAvailableOffline failed", err); + const reason = + (err as { response?: { data?: { message?: string } } })?.response?.data + ?.message ?? (err instanceof Error ? err.message : String(err)); notifications.show({ - message: t("Failed to make page available offline"), + message: `${t("Failed to make page available offline")}: ${reason}`, color: "red", }); } diff --git a/apps/client/src/features/user/hooks/use-current-user.ts b/apps/client/src/features/user/hooks/use-current-user.ts index 3d6b289d..140c51dd 100644 --- a/apps/client/src/features/user/hooks/use-current-user.ts +++ b/apps/client/src/features/user/hooks/use-current-user.ts @@ -2,9 +2,19 @@ import { useQuery, UseQueryResult } from "@tanstack/react-query"; import { getMyInfo } from "@/features/user/services/user-service"; import { ICurrentUser } from "@/features/user/types/user.types"; +/** + * Centralized React Query key factory for current-user queries. This hook and + * the offline warm path (features/offline/make-offline.ts) share it so the + * runtime key can never silently drift, and the OFFLINE_PERSIST_ROOTS guard + * test can assert the persisted "currentUser" root maps to a real factory. + */ +export const userKeys = { + currentUser: () => ["currentUser"] as const, +}; + export default function useCurrentUser(): UseQueryResult { return useQuery({ - queryKey: ["currentUser"], + queryKey: userKeys.currentUser(), queryFn: async () => { return await getMyInfo(); }, diff --git a/apps/server/src/integrations/environment/environment.service.spec.ts b/apps/server/src/integrations/environment/environment.service.spec.ts index 565755de..34157b73 100644 --- a/apps/server/src/integrations/environment/environment.service.spec.ts +++ b/apps/server/src/integrations/environment/environment.service.spec.ts @@ -60,22 +60,11 @@ describe('EnvironmentService', () => { }); describe('isSwaggerEnabled', () => { - it('is true for "true"', () => { - expect(makeService({ SWAGGER_ENABLED: 'true' }).isSwaggerEnabled()).toBe( - true, - ); - }); - - it('is true case-insensitively for "TRUE"', () => { - expect(makeService({ SWAGGER_ENABLED: 'TRUE' }).isSwaggerEnabled()).toBe( - true, - ); - }); - - it('is true for mixed-case "True"', () => { - expect(makeService({ SWAGGER_ENABLED: 'True' }).isSwaggerEnabled()).toBe( - true, - ); + // Case-insensitive: "true" in any casing enables Swagger. + it.each(['true', 'TRUE', 'True'])('is true for "%s"', (value) => { + expect( + makeService({ SWAGGER_ENABLED: value }).isSwaggerEnabled(), + ).toBe(true); }); it('defaults to false when absent', () => { diff --git a/docs/mobile-app-plan.md b/docs/mobile-app-plan.md index 8bdc6170..faf05d43 100644 --- a/docs/mobile-app-plan.md +++ b/docs/mobile-app-plan.md @@ -31,9 +31,9 @@ нативную оболочку (iOS + Android из одного кода), добавить нативные плагины (push, биометрия, share, файлы). Эволюция в гибрид (нативная навигация + WebView-редактор) делается потом инкрементально, без переписывания. -6. **Оффлайн-будущее уже заложено** (Yjs + `y-indexeddb`). Детальный план — - в [offline-sync-plan.md](offline-sync-plan.md); мобильное приложение этот - план переиспользует, а не дублирует. +6. **Оффлайн-будущее уже заложено** (Yjs + `y-indexeddb`). Детальный план + оффлайн-синхронизации (этапы M0…M4) ведётся отдельно; мобильное приложение + этот план переиспользует, а не дублирует. 7. **Главный блокер — не технический, а лицензионный.** AGPL форка несовместима с условиями App Store, если зашивать веб-клиент в бинарник: DRM/usage-rules Apple = «дополнительные ограничения», запрещённые AGPLv3 §10. Развязки — @@ -133,8 +133,8 @@ одновременно, без второй команды. - Адаптивная вёрстка уже есть (см. §2.3) — переверстывать под телефон с нуля не нужно; работа смещается в нативную обвязку. -- Оффлайн-будущее подготовлено (Yjs + `y-indexeddb`); см. - [offline-sync-plan.md](offline-sync-plan.md). +- Оффлайн-будущее подготовлено (Yjs + `y-indexeddb`); оффлайн-синхронизация + ведётся отдельным планом (этапы M0…M4). - Когда упрётесь в UX отдельных экранов — их по одному выносят в нативную оболочку, оставив редактор в WebView. То есть B → C делается инкрементально. @@ -322,7 +322,7 @@ aggregation» — не катит: зашитый бандл это комбин копия и автослияние правок работают, в том числе в WebView. - «Полностью онлайн» — это всё вокруг тела (навигация, заголовки, комментарии, CRUD, вложения, авторизация). Их оффлайн-синхронизация описана отдельным - планом с этапами M0…M4 — см. [offline-sync-plan.md](offline-sync-plan.md). + планом с этапами M0…M4. - Мобильное приложение **переиспользует** этот план, а не строит оффлайн заново. Нюанс Android: System WebView под нехваткой места может чистить хранилище → для оффлайна, возможно, понадобится дублировать критичные данные в нативное @@ -338,7 +338,7 @@ aggregation» — не катит: зашитый бандл это комбин Keystore + Bearer (рекомендуется) или попытка работать через cookie в WebView? - **Q3.** Push: APNs + FCM сразу или iOS-first? - **Q4.** Подключать ли OpenAPI/Swagger для генерации мобильного клиента? -- **Q5.** Когда включать оффлайн (M0…M4 из offline-sync-plan.md) относительно +- **Q5.** Когда включать оффлайн (этапы M0…M4) относительно первого мобильного релиза? - **Q6.** iOS-дистрибуция при AGPL (§9): App Store через `server.url` (онлайн-клиент, без зашитого AGPL), PWA или sideload/EU-маркетплейсы? Этот diff --git a/docs/mobile-bootstrap.md b/docs/mobile-bootstrap.md index d5869646..8da35a1d 100644 --- a/docs/mobile-bootstrap.md +++ b/docs/mobile-bootstrap.md @@ -20,8 +20,8 @@ mobile app for Gitmost, per the first-step checklist in as `NetworkOnly` — offline reads are served by the persisted TanStack Query cache (IndexedDB) and `y-indexeddb` for the page Yjs doc, not by an SW HTTP cache. This lets the existing responsive web UI be installed and run as a - Progressive Web App. See [docs/offline-sync-plan.md](./offline-sync-plan.md) for - the full offline/sync design. + Progressive Web App. The offline/sync design (stages M0…M4) is summarized in + [mobile-app-plan.md](./mobile-app-plan.md). - **Backend mobile auth**: opt-in token return from the login flow. The login request accepts a `returnToken` flag (must be sent as a JSON boolean) that makes the server include the auth token in the response body, and the server already