From 57b77c35e53f3d69dfbe2fd8b03325f05f6b14a8 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Mon, 29 Jun 2026 14:31:10 +0300 Subject: [PATCH] fix(offline): report editor-warm failures honestly; cover failure labels; align docs (F1-F3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: warmPageYdoc now returns didSync (true only on the real 'synced' event, false on the 8s timeout / error, which are now logged). The tree menu gates the 'available offline' success toast on result.ok AND didSync, and pushes 'editor' into the failed set otherwise — so a page whose Yjs body never warmed is no longer reported as fully offline-available. F2: tests for the page/space/currentUser failure labels and the didSync true/false paths. F3: correct the mobile-app-plan / mobile-bootstrap present-state sections to match shipped code (Capacitor, CORS allowlist, Swagger, offline reading, /l in the SW denylist). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/features/offline/make-offline.test.ts | 108 +++++++++++++++++- .../src/features/offline/make-offline.ts | 36 ++++-- .../tree/components/space-tree-node-menu.tsx | 25 ++-- docs/mobile-app-plan.md | 66 ++++++----- docs/mobile-bootstrap.md | 6 +- 5 files changed, 196 insertions(+), 45 deletions(-) diff --git a/apps/client/src/features/offline/make-offline.test.ts b/apps/client/src/features/offline/make-offline.test.ts index 76d5a8c4..6d9ec54f 100644 --- a/apps/client/src/features/offline/make-offline.test.ts +++ b/apps/client/src/features/offline/make-offline.test.ts @@ -326,6 +326,99 @@ describe("makePageAvailableOffline", () => { errorSpy.mockRestore(); }); + + it("records 'page' when the central document fetch (getPageById) rejects", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + // The central page document fetch fails (the most realistic failure). + (getPageById as ReturnType).mockRejectedValue( + new Error("network"), + ); + (getPageBreadcrumbs as ReturnType).mockResolvedValue([]); + (getSidebarPages as ReturnType).mockResolvedValue({ + items: [], + meta: { nextCursor: null }, + }); + (getPageComments as ReturnType).mockResolvedValue({ + items: [], + meta: { nextCursor: null }, + }); + + const result = await makePageAvailableOffline({ + pageId: "uuid-1", + spaceId: "space-uuid", + }); + + // With no page document, the space step is skipped (no slug), so the only + // failure label is "page". + expect(result.ok).toBe(false); + expect(result.failed).toContain("page"); + expect(result.failed).not.toContain("space"); + expect(errorSpy).toHaveBeenCalled(); + + errorSpy.mockRestore(); + }); + + it("records 'space' when ONLY the space prefetch rejects", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + (getPageById as ReturnType).mockResolvedValue(okPage); + (getPageBreadcrumbs as ReturnType).mockResolvedValue([]); + (getSidebarPages as ReturnType).mockResolvedValue({ + items: [], + meta: { nextCursor: null }, + }); + (getPageComments as ReturnType).mockResolvedValue({ + items: [], + meta: { nextCursor: null }, + }); + // Fail ONLY the space prefetch (queryKey ["space", slug]); the currentUser + // and sidebar-children prefetches still resolve. + prefetchQuery.mockImplementation(async (opts: any) => { + if (opts?.queryKey?.[0] === "space") throw new Error("network"); + return undefined; + }); + + const result = await makePageAvailableOffline({ + pageId: "uuid-1", + spaceId: "space-uuid", + }); + + expect(result.ok).toBe(false); + expect(result.failed).toContain("space"); + expect(errorSpy).toHaveBeenCalled(); + + errorSpy.mockRestore(); + }); + + it("records 'currentUser' when ONLY the currentUser prefetch rejects", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + (getPageById as ReturnType).mockResolvedValue(okPage); + (getPageBreadcrumbs as ReturnType).mockResolvedValue([]); + (getSidebarPages as ReturnType).mockResolvedValue({ + items: [], + meta: { nextCursor: null }, + }); + (getPageComments as ReturnType).mockResolvedValue({ + items: [], + meta: { nextCursor: null }, + }); + // Fail ONLY the currentUser prefetch (queryKey ["currentUser"]); the space + // and sidebar-children prefetches still resolve. + prefetchQuery.mockImplementation(async (opts: any) => { + if (opts?.queryKey?.[0] === "currentUser") throw new Error("network"); + return undefined; + }); + + const result = await makePageAvailableOffline({ + pageId: "uuid-1", + spaceId: "space-uuid", + }); + + expect(result.ok).toBe(false); + expect(result.failed).toContain("currentUser"); + expect(errorSpy).toHaveBeenCalled(); + + errorSpy.mockRestore(); + }); }); describe("warmPageYdoc", () => { @@ -343,7 +436,8 @@ describe("warmPageYdoc", () => { )![1] as () => void; handler(); - await expect(promise).resolves.toBeUndefined(); + // Returns true because the real "synced" event fired. + await expect(promise).resolves.toBe(true); // Listener detached and everything cleaned up. expect(h.providerOff).toHaveBeenCalledWith("synced", expect.any(Function)); @@ -358,16 +452,24 @@ describe("warmPageYdoc", () => { expect(h.ydocDestroy).toHaveBeenCalledTimes(1); }); - it("resolves and cleans up after the timeout when synced never fires", async () => { + it("resolves false and cleans up after the timeout when synced never fires", async () => { vi.useFakeTimers(); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); const promise = warmPageYdoc("p1", "ws://x"); // Do not fire "synced"; let the 8s safety timeout settle it. await vi.advanceTimersByTimeAsync(8000); - await expect(promise).resolves.toBeUndefined(); + // Returns false (the doc never synced) and logs the timeout with the pageId. + await expect(promise).resolves.toBe(false); + expect(errorSpy).toHaveBeenCalledWith( + "warmPageYdoc: timed out before sync", + { pageId: "p1" }, + ); expect(h.providerDestroy).toHaveBeenCalledTimes(1); expect(h.idbDestroy).toHaveBeenCalledTimes(1); expect(h.ydocDestroy).toHaveBeenCalledTimes(1); + + errorSpy.mockRestore(); }); }); diff --git a/apps/client/src/features/offline/make-offline.ts b/apps/client/src/features/offline/make-offline.ts index 7e67cf70..eca52f43 100644 --- a/apps/client/src/features/offline/make-offline.ts +++ b/apps/client/src/features/offline/make-offline.ts @@ -247,16 +247,25 @@ export async function makePageAvailableOffline({ * pull the server state into IndexedDB, then tears both down once synced (or * after a timeout). Entirely wrapped in try/catch — NEVER throws. * + * Returns true ONLY when the provider's real "synced" event fired — i.e. the + * server state actually landed in IndexedDB. The timeout and failure paths + * return false (and log with the pageId) so the caller does not report a page + * as offline-available when its editor body never warmed. For a wiki the editor + * body IS the page, so a silent timeout here is a real misreport. + * * Only meaningful when online at warm time; offline it is a no-op that resolves. */ export async function warmPageYdoc( pageId: string, collabUrl: string, token?: string, -): Promise { +): Promise { let ydoc: Y.Doc | null = null; let local: IndexeddbPersistence | null = null; let remote: HocuspocusProvider | null = null; + // Flipped to true ONLY inside the real "synced" handler; the timeout/failure + // paths leave it false. Returned so the caller can record a failed editor warm. + let didSync = false; try { const documentName = pageYdocName(pageId); @@ -274,27 +283,38 @@ export async function warmPageYdoc( await new Promise((resolve) => { let settled = false; let timeoutId: ReturnType | undefined; - const finish = () => { + // `synced` is true only when called from the real "synced" handler; the + // timeout path passes false so didSync stays false on a give-up. + const finish = (synced: boolean) => { if (settled) return; settled = true; + didSync = synced; // Clear the pending timeout and detach the listener so neither leaks // after we resolve. if (timeoutId !== undefined) clearTimeout(timeoutId); try { - provider.off("synced", finish); + provider.off("synced", onSynced); } catch { // best-effort } + if (!synced) { + // Gave up before the server synced: the page body never landed in + // IndexedDB. Log with the pageId (parity with the other warm steps) + // so the caller can report the editor step as failed. + console.error("warmPageYdoc: timed out before sync", { pageId }); + } resolve(); }; + const onSynced = () => finish(true); + // Resolve once the server state has synced into the local doc... - provider.on("synced", finish); + provider.on("synced", onSynced); // ...or give up after a short timeout so we never hang. - timeoutId = setTimeout(finish, 8000); + timeoutId = setTimeout(() => finish(false), 8000); }); - } catch { - // best-effort + } catch (error) { + console.error("warmPageYdoc: warm failed", { pageId, error }); } finally { try { remote?.destroy(); @@ -312,4 +332,6 @@ export async function warmPageYdoc( // best-effort } } + + return didSync; } 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 997955ed..70937f6c 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 @@ -90,18 +90,29 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) { pageId: node.id, spaceId: node.spaceId, }); - // Best-effort: warm the page's Yjs document into IndexedDB. - await warmPageYdoc(node.id, getCollaborationUrl(), collabQuery?.token); + // Warm the page's Yjs document into IndexedDB. For a wiki the editor body + // IS the page, so this only truly succeeds when the doc actually synced; + // a timeout/failure here must NOT be reported as offline-available. + const ydocSynced = await warmPageYdoc( + node.id, + getCollaborationUrl(), + collabQuery?.token, + ); - if (result.ok) { + // Fold a failed editor warm into the failed-step set so it surfaces in the + // same error UI as the read-query failures (the editor body never landed + // in IndexedDB, so the page would open blank offline). + const failed = ydocSynced ? result.failed : [...result.failed, "editor"]; + + if (result.ok && ydocSynced) { notifications.show({ message: t("Page is now available offline") }); } else { // Partial warm — the page may still be partly usable offline, but some - // queries failed to cache, so surface it as an error rather than a - // silent success. Name the failed step(s) (AGENTS.md: errors must be - // specific, never a bare generic string); `result.failed` carries them. + // queries (or the editor body) failed to cache, so surface it as an + // error rather than a silent success. Name the failed step(s) (AGENTS.md: + // errors must be specific, never a bare generic string). notifications.show({ - message: `${t("Failed to make page available offline")}: ${result.failed.join(", ")}`, + message: `${t("Failed to make page available offline")}: ${failed.join(", ")}`, color: "red", }); } diff --git a/docs/mobile-app-plan.md b/docs/mobile-app-plan.md index faf05d43..811c64d7 100644 --- a/docs/mobile-app-plan.md +++ b/docs/mobile-app-plan.md @@ -4,7 +4,9 @@ > Контекст: gitmost — форк Docmost, чистое веб-приложение. Отдельного > мобильного (нативного/устанавливаемого) приложения **нет**. > Цель: определить путь к мобильным приложениям — **iOS обязательно, Android -> как пойдёт** — с заделом на оффлайн в будущем (оффлайн сейчас не требуется). +> как пойдёт**. Оффлайн-чтение уже реализовано (`apps/client/src/features/offline/`, +> этапы M0…M2 — персист TanStack Query в IndexedDB + Yj/`y-indexeddb` тело +> документа); полноценная двусторонняя синхронизация (этапы M3…M4) ещё впереди. Документ фиксирует, что уже есть в коде, почему путь к мобилке предопределён устройством продукта, сравнивает варианты и описывает рекомендуемый план с @@ -14,8 +16,12 @@ ## 1. TL;DR -1. **Нативного приложения нет.** В проекте отсутствуют Capacitor, React Native, - Cordova и т.п. Мобильного клиента ещё не начинали. +1. **Capacitor-обвязка заведена, собранного нативного приложения ещё нет.** В + монорепо добавлены `@capacitor/core|android|ios|cli` и корневой + `capacitor.config.ts` (бутстрап этого PR, см. §12). Сгенерированные нативные + проекты (`ios/`, `android/`) намеренно не хранятся в VCS, и сборки в App + Store / Play ещё нет — это первый шаг к мобильному клиенту, а не готовое + приложение. 2. **Адаптивная веб-версия — есть, и довольно проработанная.** Веб-клиент открывается с телефона как mobile-friendly сайт: сворачиваемый сайдбар-drawer, отдельные мобильные компоненты (история, поиск, хлебные крошки), responsive- @@ -31,9 +37,10 @@ нативную оболочку (iOS + Android из одного кода), добавить нативные плагины (push, биометрия, share, файлы). Эволюция в гибрид (нативная навигация + WebView-редактор) делается потом инкрементально, без переписывания. -6. **Оффлайн-будущее уже заложено** (Yjs + `y-indexeddb`). Детальный план - оффлайн-синхронизации (этапы M0…M4) ведётся отдельно; мобильное приложение - этот план переиспользует, а не дублирует. +6. **Оффлайн-чтение уже реализовано** (Yjs + `y-indexeddb` + персист TanStack + Query в IndexedDB, этапы M0…M2 — см. `apps/client/src/features/offline/`). + Полная двусторонняя синхронизация записи (этапы M3…M4) ведётся отдельно; + мобильное приложение этот план переиспользует, а не дублирует. 7. **Главный блокер — не технический, а лицензионный.** AGPL форка несовместима с условиями App Store, если зашивать веб-клиент в бинарник: DRM/usage-rules Apple = «дополнительные ограничения», запрещённые AGPLv3 §10. Развязки — @@ -53,10 +60,13 @@ | Ядро (редактор) | TipTap 3 (ProseMirror) + совместное редактирование на Yjs через Hocuspocus — см. [page-editor.tsx](../apps/client/src/features/editor/page-editor.tsx). | | Оффлайн-фундамент | `yjs` + `y-indexeddb` уже в зависимостях клиента (локальная CRDT-копия тела документа). | -### 2.2. Мобильного приложения нет +### 2.2. Capacitor-обвязка заведена (собранного приложения ещё нет) -В `package.json` и `apps/*/package.json` нет `capacitor`, `react-native`, -`cordova`, `expo`. Нативной оболочки в репозитории не заведено. +Корневой `package.json` содержит `@capacitor/core|android|ios` (и `@capacitor/cli` +в devDependencies), в корне лежит [capacitor.config.ts](../capacitor.config.ts). +`react-native`, `cordova`, `expo` по-прежнему не используются. Сгенерированные +нативные проекты (`ios/`, `android/`) намеренно не коммитятся — это бутстрап +оболочки, а не готовый бинарник. ### 2.3. Адаптивная веб-версия — есть @@ -158,22 +168,24 @@ Android как есть, а к iOS — только в конфигурации ## 6. Что доработать на бэкенде -Немного, но конкретно: +Часть уже сделана бутстрапом этого PR (см. §2.4), осталось нативно-инфраструктурное: -1. **Выдача токена в теле ответа для нативного хранения.** Сейчас логин кладёт - JWT только в `httpOnly`-cookie и не возвращает его в body. На мобиле - `httpOnly`-cookie между разными origin (`capacitor://localhost` ↔ API) — боль - с SameSite/CORS. Чище: мобильный логин-флоу, возвращающий JWT в ответе, чтобы - хранить его в Keychain/Keystore и слать как `Authorization: Bearer`. Сервер - уже принимает Bearer — менять надо только **выдачу**. +1. **Выдача токена в теле ответа для нативного хранения — ✅ сделано.** Логин + по-прежнему кладёт JWT в `httpOnly`-cookie, а при opt-in флаге `returnToken` + дополнительно возвращает токен в теле (`data.authToken`) для нативного хранения + в Keychain/Keystore и отправки как `Authorization: Bearer`. Сервер принимал + Bearer и раньше; добавлена именно **выдача**. Файлы: [auth.controller.ts](../apps/server/src/core/auth/auth.controller.ts). -2. **CORS.** Сейчас [`app.enableCors()`](../apps/server/src/main.ts) (L144) без - конфигурации. Под мобильные origin'ы и для безопасности задать явный whitelist. +2. **CORS — ✅ сделано.** Безусловный `app.enableCors()` заменён на явный allowlist: + [`buildCorsAllowlist`/`isOriginAllowed`](../apps/server/src/integrations/environment/cors.util.ts) + собирают доверенные origin'ы из `CORS_ALLOWED_ORIGINS` плюс нативные + WebView-origin'ы; [main.ts](../apps/server/src/main.ts) передаёт их в + `app.enableCors({ origin: callback, credentials: true })`. 3. **Push-уведомления.** Модуль `notification` уже есть — добавить регистрацию - device-token и интеграцию **APNs** (iOS) / **FCM** (Android). -4. **Опционально — OpenAPI/Swagger.** Сейчас спецификации нет; добавить - `@nestjs/swagger` дёшево и сильно ускорит мобильную разработку - (типизированный клиент). + device-token и интеграцию **APNs** (iOS) / **FCM** (Android). (Ещё не сделано.) +4. **OpenAPI/Swagger — ✅ сделано (opt-in).** Подключён `@nestjs/swagger`; Swagger + UI доступен на `/api/docs` за флагом `SWAGGER_ENABLED` (по умолчанию выключен), + что даёт авто-генерацию типизированного клиента. --- @@ -316,13 +328,15 @@ aggregation» — не катит: зашитый бандл это комбин ## 10. Оффлайн в будущем -Оффлайн сейчас не требуется, но позиция хорошая: +Оффлайн-**чтение** уже реализовано (этапы M0…M2 этого PR), позиция хорошая: - Тело документа уже редактируется через Yjs (CRDT) + `y-indexeddb` — локальная копия и автослияние правок работают, в том числе в WebView. -- «Полностью онлайн» — это всё вокруг тела (навигация, заголовки, комментарии, - CRUD, вложения, авторизация). Их оффлайн-синхронизация описана отдельным - планом с этапами M0…M4. +- Чтение «вокруг тела» (навигация, заголовки, комментарии, дерево, текущий + пользователь) теперь читается оффлайн из персистентного кэша TanStack Query в + IndexedDB; см. `apps/client/src/features/offline/` (в т.ч. `make-offline.ts` — + ручной прогрев страницы в оффлайн). Полная **двусторонняя** синхронизация + записи (этапы M3…M4) ещё впереди. - Мобильное приложение **переиспользует** этот план, а не строит оффлайн заново. Нюанс Android: System WebView под нехваткой места может чистить хранилище → для оффлайна, возможно, понадобится дублировать критичные данные в нативное diff --git a/docs/mobile-bootstrap.md b/docs/mobile-bootstrap.md index 8da35a1d..1bb4a45e 100644 --- a/docs/mobile-bootstrap.md +++ b/docs/mobile-bootstrap.md @@ -15,8 +15,10 @@ mobile app for Gitmost, per the first-step checklist in `main.tsx` and skipped inside the Capacitor native WebView. The SW precaches the app shell (`globPatterns` js/css/html/...) and serves `navigateFallback: "index.html"` for SPA routes, with `navigateFallbackDenylist` excluding the - server-owned routes `/api`, `/collab`, `/socket.io`, `/share/`, `/mcp`, and - `/robots.txt`. `runtimeCaching` keeps `/collab`, `/socket.io`, and all `/api` + server-owned routes `/api`, `/collab`, `/socket.io`, `/share/`, `/mcp`, `/l` + (the vanity short-link `l/:alias`, excluded from the `/api` global prefix and + resolved server-side), and `/robots.txt`. `runtimeCaching` keeps `/collab`, + `/socket.io`, and all `/api` 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