diff --git a/AGENTS.md b/AGENTS.md index 064c3ccd..adbd3a42 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -463,6 +463,7 @@ Vite SPA. Code is organized by feature under `apps/client/src/features/*` (mirro - The editor is Tiptap; shared node/mark extensions live in `packages/editor-ext` and are imported by **both the client and the server** (collaboration, schema, `canonicalizeFootnotes`) — editor schema changes often need to be made in `editor-ext`, not just the client. Server-side markdown import/export no longer lives in `editor-ext`: it goes through the canonical converter (#345, see below). The ProseMirror↔Markdown converter and its Docmost schema mirror now live in a SINGLE package, `@docmost/prosemirror-markdown` (#293), consumed by `mcp`, `git-sync`, and `apps/server` (#345) — do NOT reintroduce a per-package copy. `editor-ext` is the upstream source of the Tiptap schema; the package's `docmost-schema.ts` mirrors it and a serializer-contract test (`packages/prosemirror-markdown/test/serializer-contract.test.ts`) guards the boundary (every schema node must have a converter case), so a drift surfaces as a failing test rather than silent divergence. For the converter's property-testing and counterexample→fixture process (P1–P4 invariants, the `PROPERTY_SEED`/`PROPERTY_NUM_RUNS` knobs, and the nightly fuzz workflow), see `packages/prosemirror-markdown/README.md`. - API access goes through `apps/client/src/lib/api-client.ts` (axios). The `@` alias maps to `apps/client/src`. - Runtime config is injected at build time by `vite.config.ts` via `define` (`APP_URL`, `COLLAB_URL`, `APP_VERSION`, …) — these come from the root `.env`, not from `import.meta.env`. +- The build also emits `client/dist/version.json` (`{"version": …}`) from a small `vite.config.ts` plugin using the **same** `appVersion` that feeds `define.APP_VERSION`, so the file and the baked-in bundle version are identical by construction. The server reads it at startup (`ws.gateway.ts` via `readClientBuildVersion`/`resolveClientDistPath`) and announces it to each socket on connect (`app-version` event) so a tab left open across a redeploy can guard-reload before hitting a stale chunk (version-coherence). No runtime env / Dockerfile change — the file already ships in `client/dist`; missing/empty file ⇒ feature inert. ## Conventions diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b44878a..b52a8364 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -117,6 +117,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Open tabs pick up a new deploy on their own.** After the server is + redeployed while a tab is left open for hours, the tab now learns the new + build version over the existing WebSocket (announced per-connect, so a natural + reconnect delivers it) and shows a "A new version is available" banner with an + Update button. To avoid dropping a half-written comment or form, the tab is + not reloaded when you merely switch away from it; instead it auto-reloads at + the next safe point — the next in-app navigation (or immediately if you click + Update) — before it can hit a stale lazy-loaded chunk. At most one automatic + reload happens per browser session (shared with the existing chunk-load + recovery), so a permanent version skew degrades to the banner rather than a + reload loop. When the build carries no version info the feature stays inert. + - **Place several images side by side in a row.** A new "Inline (side by side)" alignment mode in the image bubble menu renders consecutive inline images as a row that wraps onto the next line on narrow screens. The row is diff --git a/apps/client/src/components/chunk-load-error-boundary.tsx b/apps/client/src/components/chunk-load-error-boundary.tsx index ed8014a4..6080017b 100644 --- a/apps/client/src/components/chunk-load-error-boundary.tsx +++ b/apps/client/src/components/chunk-load-error-boundary.tsx @@ -1,7 +1,11 @@ import { ReactNode } from "react"; import { ErrorBoundary } from "react-error-boundary"; import { Button, Center, Stack, Text } from "@mantine/core"; -import { hasAutoReloaded, markAutoReloaded } from "@/lib/reload-guard"; +import { + hasAutoReloaded, + markAutoReloaded, + recordReloadBreadcrumb, +} from "@/lib/reload-guard"; // Heuristic detection of a failed dynamic import. Since the code-splitting work, // every route (plus Aside / AiChatWindow) is React.lazy: when a new deploy @@ -30,6 +34,9 @@ function handleError(error: unknown) { // fall through to the manual recovery UI below rather than risk a loop. if (hasAutoReloaded()) return; if (!markAutoReloaded()) return; + // Trace before the reload clears the console (same diagnostic breadcrumb the + // proactive version-coherence path writes, tagged with this path). + recordReloadBreadcrumb({ path: "chunk-boundary" }); window.location.reload(); } diff --git a/apps/client/src/features/user/guarded-reload.test.tsx b/apps/client/src/features/user/guarded-reload.test.tsx index 8b1c984b..72ca5234 100644 --- a/apps/client/src/features/user/guarded-reload.test.tsx +++ b/apps/client/src/features/user/guarded-reload.test.tsx @@ -1,4 +1,6 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { render, act, cleanup } from "@testing-library/react"; +import { MemoryRouter, useNavigate } from "react-router-dom"; // Mocks for the dirty shell's side-effecting collaborators. vi.mock("@mantine/notifications", () => ({ @@ -8,12 +10,15 @@ vi.mock("@/i18n.ts", () => ({ default: { t: (k: string) => k } })); vi.mock("@/lib/reload-guard", () => ({ hasAutoReloaded: vi.fn(() => false), markAutoReloaded: vi.fn(() => true), + recordReloadBreadcrumb: vi.fn(), + takeReloadBreadcrumb: vi.fn(() => null), })); import { notifications } from "@mantine/notifications"; import { hasAutoReloaded, markAutoReloaded } from "@/lib/reload-guard"; import { triggerGuardedReload, + useVersionReloadOnNavigation, __resetGuardedReloadForTests, } from "./guarded-reload"; @@ -28,9 +33,28 @@ const mockMarkAutoReloaded = markAutoReloaded as unknown as ReturnType< let reload: ReturnType; let visibility: DocumentVisibilityState; -function setVisibility(state: DocumentVisibilityState) { - visibility = state; - document.dispatchEvent(new Event("visibilitychange")); +// Test harness mounted inside a router: it installs the navigation hook and +// exposes `navigate` so a test can drive an in-app router navigation. +let doNavigate: (to: string) => void; +function Harness() { + useVersionReloadOnNavigation(); + const navigate = useNavigate(); + doNavigate = navigate; + return null; +} + +function mountHarness() { + render( + + + , + ); +} + +function navigateTo(path: string) { + act(() => { + doNavigate(path); + }); } beforeEach(() => { @@ -55,10 +79,11 @@ beforeEach(() => { }); afterEach(() => { + cleanup(); vi.unstubAllGlobals(); }); -describe("triggerGuardedReload", () => { +describe("triggerGuardedReload (variant C)", () => { it("noop when versions match: no banner, no reload", () => { triggerGuardedReload("test-A"); expect(show).not.toHaveBeenCalled(); @@ -72,16 +97,9 @@ describe("triggerGuardedReload", () => { expect(reload).not.toHaveBeenCalled(); }); - it("hidden tab on a real mismatch reloads immediately, no banner", () => { - visibility = "hidden"; + it("real mismatch shows the banner but does NOT reload immediately", () => { + mountHarness(); triggerGuardedReload("test-B"); - expect(reload).toHaveBeenCalledTimes(1); - expect(show).not.toHaveBeenCalled(); - }); - - it("visible tab on a real mismatch shows the banner and arms a reload on hidden", () => { - triggerGuardedReload("test-B"); - // Banner shown, no immediate reload. expect(show).toHaveBeenCalledTimes(1); expect(show.mock.calls[0][0]).toMatchObject({ id: "app-version-reload", @@ -89,30 +107,83 @@ describe("triggerGuardedReload", () => { withCloseButton: true, }); expect(reload).not.toHaveBeenCalled(); + }); - // Going to the background triggers the guarded auto-reload. - setVisibility("hidden"); + it("reloads EXACTLY ONCE on the first in-app navigation after a mismatch", () => { + mountHarness(); + triggerGuardedReload("test-B"); + expect(reload).not.toHaveBeenCalled(); + + navigateTo("/next"); + expect(reload).toHaveBeenCalledTimes(1); + + // A second navigation must NOT reload again (one-shot was consumed). + navigateTo("/again"); expect(reload).toHaveBeenCalledTimes(1); }); - it("banner-only (auto-reload already spent): banner, never auto-reload", () => { - mockHasAutoReloaded.mockReturnValue(true); + it("does NOT reload on a 2nd mismatch after the first was armed/consumed (one-shot)", () => { + mountHarness(); + triggerGuardedReload("test-B"); + navigateTo("/next"); + expect(reload).toHaveBeenCalledTimes(1); + + // Another app-version mismatch arrives (reconnect): must not re-arm. + triggerGuardedReload("test-C"); + navigateTo("/again"); + expect(reload).toHaveBeenCalledTimes(1); + }); + + it("does NOT reload merely from the tab going to the background", () => { + mountHarness(); triggerGuardedReload("test-B"); - expect(show).toHaveBeenCalledTimes(1); expect(reload).not.toHaveBeenCalled(); - // Even backgrounding must not reload (no listener was armed). - setVisibility("hidden"); + visibility = "hidden"; + act(() => { + document.dispatchEvent(new Event("visibilitychange")); + }); + expect(reload).not.toHaveBeenCalled(); + }); + + it("a hidden-at-receipt tab is also NOT reloaded immediately (variant C uniform); reloads on next navigation", () => { + visibility = "hidden"; + mountHarness(); + triggerGuardedReload("test-B"); + expect(reload).not.toHaveBeenCalled(); + expect(show).toHaveBeenCalledTimes(1); + + navigateTo("/next"); + expect(reload).toHaveBeenCalledTimes(1); + }); + + it("the banner's Update button reloads immediately", () => { + triggerGuardedReload("test-B"); + const message = show.mock.calls[0][0].message as { + props: { onClick: () => void }; + }; + message.props.onClick(); + expect(reload).toHaveBeenCalledTimes(1); + }); + + it("banner-only (auto-reload already spent): banner, never auto-reload on navigation", () => { + mockHasAutoReloaded.mockReturnValue(true); + mountHarness(); + triggerGuardedReload("test-B"); + expect(show).toHaveBeenCalledTimes(1); + + navigateTo("/next"); expect(reload).not.toHaveBeenCalled(); }); it("does NOT reload when the flag write fails; falls back to the banner", () => { mockMarkAutoReloaded.mockReturnValue(false); - visibility = "hidden"; + mountHarness(); triggerGuardedReload("test-B"); + navigateTo("/next"); expect(reload).not.toHaveBeenCalled(); - // performAutoReload falls back to showing the banner. - expect(show).toHaveBeenCalledTimes(1); + // performAutoReload falls back to showing the banner (initial + fallback). + expect(show).toHaveBeenCalled(); }); it("is idempotent within a tab-load: repeated emits do not stack banners", () => { diff --git a/apps/client/src/features/user/guarded-reload.tsx b/apps/client/src/features/user/guarded-reload.tsx index d483f0e9..82b81d14 100644 --- a/apps/client/src/features/user/guarded-reload.tsx +++ b/apps/client/src/features/user/guarded-reload.tsx @@ -1,23 +1,43 @@ +import { useEffect, useRef } from "react"; +import { useLocation } from "react-router-dom"; import { Button } from "@mantine/core"; import { notifications } from "@mantine/notifications"; import i18n from "@/i18n.ts"; -import { hasAutoReloaded, markAutoReloaded } from "@/lib/reload-guard"; +import { + hasAutoReloaded, + markAutoReloaded, + recordReloadBreadcrumb, + takeReloadBreadcrumb, +} from "@/lib/reload-guard"; import { decideVersionAction } from "@/features/user/version-coherence"; // Dirty shell around the pure `decideVersionAction`: it reads globals -// (APP_VERSION, document.visibilityState), touches sessionStorage via the -// shared reload-guard, and drives the Mantine notification. Kept separate from -// the pure module so the decision stays unit-testable without a DOM. +// (APP_VERSION), touches sessionStorage via the shared reload-guard, drives the +// Mantine notification, and arms the router-navigation reload hook. Kept +// separate from the pure module so the decision stays unit-testable without a +// DOM. // One fixed id so repeated app-version signals (e.g. every reconnect) update a // single banner instead of stacking a new one each time. const BANNER_ID = "app-version-reload"; // Module-level idempotency for the current tab-load: once a mismatch has been -// handled we don't re-arm the visibility listener or re-show the banner on +// handled we don't re-arm the navigation reload or re-show the banner on // subsequent app-version emits. let handled = false; +// Variant C: on a real mismatch we do NOT reload the tab when it merely goes to +// the background (that would silently drop a half-written comment/form). Instead +// we arm a one-shot reload for the NEXT in-app router navigation — a point where +// the user is already leaving the current page, so an in-app navigation would +// discard that unsaved component-state anyway and the reload adds no extra loss. +let pendingNavReload = false; + +// Remembered from the last detected mismatch for the pre-reload breadcrumb and +// the (already-visible) banner. +let lastServerVersion = ""; +let lastClientVersion = ""; + // Read the build version baked into THIS bundle. The `typeof` guard avoids a // ReferenceError where the `APP_VERSION` global is absent (e.g. under vitest, // where Vite's `define` did not run) — an unknown client version makes the @@ -35,6 +55,16 @@ function performAutoReload(): void { showReloadBanner(); return; } + // Trace right before the reload (which clears the console): a persistent + // breadcrumb + a log line so the auto-reload is observable in a field report. + recordReloadBreadcrumb({ + path: "proactive", + serverVersion: lastServerVersion, + clientVersion: lastClientVersion, + }); + console.warn( + `[version-coherence] auto-reloading: client=${lastClientVersion} -> server=${lastServerVersion}`, + ); window.location.reload(); } @@ -54,13 +84,15 @@ function showReloadBanner(): void { /** * Handle a server `app-version` announcement: compare it to this bundle's - * version and, on a real mismatch, do a guarded reload. + * version and, on a real mismatch, show the banner and arm a guarded reload for + * the next in-app navigation (variant C). * - * - hidden tab → reload immediately (nobody is looking). - * - visible tab → show the banner AND self-reload the moment the - * tab goes to the background (or on the button). - * - auto-reload already used / storage error → banner only (no auto-reload), - * so there is at most one automatic reload per session (loop safety). + * - real mismatch (first this session) → banner + arm navigation reload. The + * banner's "Update" button reloads immediately (same one-shot guard). The tab + * is NOT reloaded on visibility change. + * - auto-reload already used / storage error → banner only (no arm), so there is + * at most one automatic reload per session (loop safety). + * - in sync / unknown version → noop (fail-safe). */ export function triggerGuardedReload( rawServerVersion: string | undefined | null, @@ -79,11 +111,14 @@ export function triggerGuardedReload( }); if (action === "noop") return; - // Idempotent per tab-load: don't stack banners or re-arm the listener across - // repeated emits (reconnects) once we've already acted. + // Idempotent per tab-load: don't re-arm or re-stack the banner across repeated + // emits (reconnects) once we've already acted. if (handled) return; handled = true; + lastServerVersion = serverVersion; + lastClientVersion = clientVersion; + if (action === "banner") { // Entered banner-only (permanent skew, node oscillation, or spent // auto-reload). Log for diagnosability; show the manual banner. @@ -95,22 +130,58 @@ export function triggerGuardedReload( return; } - // action === "reload" - if (document.visibilityState === "hidden") { - // Covers tabs that are already backgrounded at the moment the signal - // arrives — reload them right away. - performAutoReload(); - return; - } - + // action === "reload" (variant C): show the banner and defer the auto-reload + // to the next in-app navigation instead of reloading now / on visibility. showReloadBanner(); - const onHidden = () => { - if (document.visibilityState === "hidden") performAutoReload(); - }; - document.addEventListener("visibilitychange", onHidden, { once: true }); + pendingNavReload = true; } -// Test-only: reset the module-level idempotency latch between cases. +/** + * Consume the armed one-shot navigation reload, if any. Called by + * `useVersionReloadOnNavigation` on each in-app router navigation. + */ +export function consumeNavigationReload(): void { + if (!pendingNavReload) return; + pendingNavReload = false; + performAutoReload(); +} + +/** + * Hook (mounted inside the Router) that fires the armed one-shot reload on the + * NEXT in-app router navigation after a version mismatch. Skips the initial + * render so it only reacts to real navigations, not the first location. + */ +export function useVersionReloadOnNavigation(): void { + const location = useLocation(); + const firstRender = useRef(true); + useEffect(() => { + if (firstRender.current) { + firstRender.current = false; + return; + } + consumeNavigationReload(); + }, [location.key]); +} + +/** + * Surface (log once) the breadcrumb left by an auto-reload in the previous page + * load — the reload cleared the console, so this makes a "tab reloaded itself" + * report diagnosable. Call once on app startup. + */ +export function surfacePreviousReloadBreadcrumb(): void { + const crumb = takeReloadBreadcrumb(); + if (!crumb) return; + console.info( + `[version-coherence] previous auto-reload: path=${crumb.path} ` + + `client=${crumb.clientVersion ?? ""} -> server=${crumb.serverVersion ?? ""} ` + + `at=${new Date(crumb.at).toISOString()}`, + ); +} + +// Test-only: reset module-level latches between cases. export function __resetGuardedReloadForTests(): void { handled = false; + pendingNavReload = false; + lastServerVersion = ""; + lastClientVersion = ""; } diff --git a/apps/client/src/features/user/user-provider.tsx b/apps/client/src/features/user/user-provider.tsx index 7d3a591d..08af6f65 100644 --- a/apps/client/src/features/user/user-provider.tsx +++ b/apps/client/src/features/user/user-provider.tsx @@ -13,7 +13,11 @@ import { useCollabToken } from "@/features/auth/queries/auth-query.tsx"; import { Error404 } from "@/components/ui/error-404.tsx"; import { queryClient } from "@/main.tsx"; import { makeConnectHandler } from "@/features/user/connect-resync.ts"; -import { triggerGuardedReload } from "@/features/user/guarded-reload.tsx"; +import { + triggerGuardedReload, + useVersionReloadOnNavigation, + surfacePreviousReloadBreadcrumb, +} from "@/features/user/guarded-reload.tsx"; import type { AppVersionSocketPayload } from "@/features/user/version-coherence.ts"; export function UserProvider({ children }: React.PropsWithChildren) { @@ -24,6 +28,16 @@ export function UserProvider({ children }: React.PropsWithChildren) { // fetch collab token on load const { data: collab } = useCollabToken(); + // version-coherence: fire the armed one-shot reload on the next in-app + // navigation (variant C — a safe point, not on tab backgrounding). + useVersionReloadOnNavigation(); + + // Surface any breadcrumb left by an auto-reload in the previous page load + // (the reload cleared the console) so a field report stays diagnosable. + useEffect(() => { + surfacePreviousReloadBreadcrumb(); + }, []); + useEffect(() => { if (isLoading || isError) { return; diff --git a/apps/client/src/lib/reload-guard.test.ts b/apps/client/src/lib/reload-guard.test.ts index 1c549af4..4afa7d2b 100644 --- a/apps/client/src/lib/reload-guard.test.ts +++ b/apps/client/src/lib/reload-guard.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { hasAutoReloaded, markAutoReloaded } from "./reload-guard"; +import { + hasAutoReloaded, + markAutoReloaded, + recordReloadBreadcrumb, + takeReloadBreadcrumb, +} from "./reload-guard"; const FLAG = "chunk-reload-attempted"; @@ -51,4 +56,42 @@ describe("reload-guard", () => { vi.unstubAllGlobals(); } }); + + it("records and then takes a breadcrumb once (cleared on read)", () => { + recordReloadBreadcrumb({ + path: "proactive", + serverVersion: "test-B", + clientVersion: "test-A", + }); + const crumb = takeReloadBreadcrumb(); + expect(crumb).toMatchObject({ + path: "proactive", + serverVersion: "test-B", + clientVersion: "test-A", + }); + expect(typeof crumb?.at).toBe("number"); + // Cleared on read → a second take returns null. + expect(takeReloadBreadcrumb()).toBeNull(); + }); + + it("takeReloadBreadcrumb returns null when nothing was recorded", () => { + expect(takeReloadBreadcrumb()).toBeNull(); + }); + + it("recordReloadBreadcrumb swallows a storage-write error (diagnostics only)", () => { + vi.stubGlobal("sessionStorage", { + getItem: () => null, + setItem: () => { + throw new Error("storage disabled"); + }, + removeItem: () => {}, + }); + try { + expect(() => + recordReloadBreadcrumb({ path: "chunk-boundary" }), + ).not.toThrow(); + } finally { + vi.unstubAllGlobals(); + } + }); }); diff --git a/apps/client/src/lib/reload-guard.ts b/apps/client/src/lib/reload-guard.ts index 8b7e2c49..754827c1 100644 --- a/apps/client/src/lib/reload-guard.ts +++ b/apps/client/src/lib/reload-guard.ts @@ -40,3 +40,51 @@ export function markAutoReloaded(): boolean { return false; } } + +// Diagnostic breadcrumb for an automatic reload. Written right before +// window.location.reload() (which clears the console) and read back on the next +// page load, so a "the tab reloaded itself / it's looping" field report is +// diagnosable: which path fired (proactive version-coherence vs the reactive +// chunk-load boundary) and which version pair triggered it. sessionStorage +// survives a same-tab reload, unlike the console. +const RELOAD_BREADCRUMB_KEY = "reload-breadcrumb"; + +export type ReloadBreadcrumb = { + path: "proactive" | "chunk-boundary"; + serverVersion?: string; + clientVersion?: string; + at: number; +}; + +/** + * Persist a best-effort breadcrumb just before an automatic reload. Failures + * (storage unavailable) are swallowed — this is diagnostics only and must never + * block or alter the reload decision. + */ +export function recordReloadBreadcrumb( + entry: Omit, +): void { + try { + sessionStorage.setItem( + RELOAD_BREADCRUMB_KEY, + JSON.stringify({ ...entry, at: Date.now() }), + ); + } catch { + // best-effort diagnostics only + } +} + +/** + * Read and clear the breadcrumb left by an auto-reload in the previous page + * load. Cleared on read so it surfaces exactly once per reload. + */ +export function takeReloadBreadcrumb(): ReloadBreadcrumb | null { + try { + const raw = sessionStorage.getItem(RELOAD_BREADCRUMB_KEY); + if (!raw) return null; + sessionStorage.removeItem(RELOAD_BREADCRUMB_KEY); + return JSON.parse(raw) as ReloadBreadcrumb; + } catch { + return null; + } +}