Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| d36021a111 | |||
| 12ed3a0332 |
@@ -463,7 +463,6 @@ 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`, `apps/server` (#345), and `apps/client` (#347) — do NOT reintroduce a per-package copy. The client uses the package's `browser` entry (`@docmost/prosemirror-markdown/browser`): markdown paste (`markdown-clipboard.ts`), copy-as-markdown, and AI-chat rendering now all go through the canonical converter, so the hand-written `marked`/`turndown` markdown layer that used to live in `editor-ext` was deleted (#347). The browser entry runs the HTML→DOM stage on the native `DOMParser`, so jsdom stays out of the client bundle. `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
|
||||
|
||||
|
||||
@@ -142,18 +142,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
snapshots switched from a fixed interval to a trailing idle-flush with a
|
||||
max-wait ceiling, and a boundary snapshot is pinned whenever the editing source
|
||||
changes (e.g. a person's edits followed by the AI agent). (#370)
|
||||
- **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 5-minute window, shared with the existing chunk-load
|
||||
recovery, so a permanent version skew degrades to the banner rather than a
|
||||
reload loop while a second deploy in the same tab still recovers. When the
|
||||
build carries no version info the feature stays inert. (#481)
|
||||
|
||||
- **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
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
{
|
||||
"A new version is available": "A new version is available",
|
||||
"Account": "Account",
|
||||
"Active": "Active",
|
||||
"Add": "Add",
|
||||
@@ -240,6 +239,8 @@
|
||||
"Comment re-opened successfully": "Comment re-opened successfully",
|
||||
"Comment unresolved successfully": "Comment unresolved successfully",
|
||||
"Failed to resolve comment": "Failed to resolve comment",
|
||||
"Failed to re-open comment": "Failed to re-open comment",
|
||||
"Comment no longer exists": "Comment no longer exists",
|
||||
"Resolve comment": "Resolve comment",
|
||||
"Unresolve comment": "Unresolve comment",
|
||||
"Resolve Comment Thread": "Resolve Comment Thread",
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
{
|
||||
"A new version is available": "Доступна новая версия",
|
||||
"Account": "Аккаунт",
|
||||
"Active": "Активный",
|
||||
"Add": "Добавить",
|
||||
@@ -240,6 +239,8 @@
|
||||
"Comment re-opened successfully": "Комментарий успешно открыт повторно",
|
||||
"Comment unresolved successfully": "Комментарий успешно переведён в нерешённые",
|
||||
"Failed to resolve comment": "Не удалось разрешить комментарий",
|
||||
"Failed to re-open comment": "Не удалось переоткрыть комментарий",
|
||||
"Comment no longer exists": "Комментарий больше не существует",
|
||||
"Resolve comment": "Решить комментарий",
|
||||
"Unresolve comment": "Снять статус решённого с комментария",
|
||||
"Resolve Comment Thread": "Решить ветку комментариев",
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { isChunkLoadError } from "./chunk-load-error-boundary";
|
||||
import { isChunkLoadError, shouldAutoReload } from "./chunk-load-error-boundary";
|
||||
|
||||
// The detector decides whether a caught render error is a stale-deploy chunk-404
|
||||
// (→ auto-reload to fetch the new manifest) vs a genuine app error (→ generic
|
||||
@@ -35,3 +35,31 @@ describe("isChunkLoadError", () => {
|
||||
expect(isChunkLoadError(err)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// The window gate replaces the old one-shot flag: it must permit recovery across
|
||||
// several deploys in one tab (each > window apart) while still stopping an infinite
|
||||
// reload loop when a lazy chunk is permanently broken (a second failure < window).
|
||||
describe("shouldAutoReload", () => {
|
||||
const WINDOW = 5 * 60 * 1000;
|
||||
const NOW = 1_000_000_000_000;
|
||||
|
||||
it("allows a reload when we have never auto-reloaded", () => {
|
||||
expect(shouldAutoReload(NOW, null, WINDOW)).toBe(true);
|
||||
});
|
||||
|
||||
it("allows a reload when the last one was 6 minutes ago (outside the window)", () => {
|
||||
expect(shouldAutoReload(NOW, NOW - 6 * 60 * 1000, WINDOW)).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks a reload when the last one was 1 minute ago (inside the window)", () => {
|
||||
expect(shouldAutoReload(NOW, NOW - 1 * 60 * 1000, WINDOW)).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks a reload exactly at the window boundary (not strictly older)", () => {
|
||||
expect(shouldAutoReload(NOW, NOW - WINDOW, WINDOW)).toBe(false);
|
||||
});
|
||||
|
||||
it("allows a reload when the stored timestamp is unparseable (NaN)", () => {
|
||||
expect(shouldAutoReload(NOW, NaN, WINDOW)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,11 +1,26 @@
|
||||
import { ReactNode } from "react";
|
||||
import { ErrorBoundary } from "react-error-boundary";
|
||||
import { Button, Center, Stack, Text } from "@mantine/core";
|
||||
import {
|
||||
hasAutoReloaded,
|
||||
markAutoReloaded,
|
||||
recordReloadBreadcrumb,
|
||||
} from "@/lib/reload-guard";
|
||||
|
||||
// sessionStorage key holding the epoch-ms timestamp of the last automatic reload.
|
||||
const RELOAD_AT_KEY = "chunk-reload-at";
|
||||
// Allow at most one automatic reload per this window. A stale-deploy 404 is cured
|
||||
// by a single reload, so anything inside the window is treated as a reload loop
|
||||
// (permanently-broken chunk) and falls through to the manual UI. A window (rather
|
||||
// than a one-shot flag) lets a SECOND deploy in the same tab's lifetime recover too.
|
||||
const RELOAD_WINDOW_MS = 5 * 60 * 1000;
|
||||
|
||||
// Pure window decision, unit-tested in isolation: auto-reload only if we have never
|
||||
// auto-reloaded (lastReloadAt null/NaN) or the last one was strictly older than the
|
||||
// window. Anything inside the window is suppressed to break an infinite reload loop.
|
||||
export function shouldAutoReload(
|
||||
now: number,
|
||||
lastReloadAt: number | null,
|
||||
windowMs: number,
|
||||
): boolean {
|
||||
if (lastReloadAt === null || Number.isNaN(lastReloadAt)) return true;
|
||||
return now - lastReloadAt > windowMs;
|
||||
}
|
||||
|
||||
// Heuristic detection of a failed dynamic import. Since the code-splitting work,
|
||||
// every route (plus Aside / AiChatWindow) is React.lazy: when a new deploy
|
||||
@@ -24,26 +39,24 @@ export function isChunkLoadError(error: unknown): boolean {
|
||||
);
|
||||
}
|
||||
|
||||
// Exported for tests: the reactive chunk-load reload decision, so the shared
|
||||
// window budget (invariant: ≤1 auto-reload per window across this path AND the
|
||||
// proactive version-coherence path) can be exercised against the real guard.
|
||||
export function handleError(error: unknown) {
|
||||
function handleError(error: unknown) {
|
||||
if (!isChunkLoadError(error)) return;
|
||||
// A stale-chunk 404 is cured by a full reload that re-fetches index.html and
|
||||
// the new chunk manifest. Auto-reload at most once per window via the SHARED
|
||||
// window-based reload guard (see @/lib/reload-guard — the same budget the
|
||||
// proactive version-coherence path consumes, so a mismatch that arrives on
|
||||
// both paths reloads at most once per window across BOTH). This recovers
|
||||
// across multiple deploys in a single tab's lifetime, yet a permanently-broken
|
||||
// lazy chunk (which would loop) is stopped after the first reload and falls
|
||||
// through to the manual recovery UI below. If the shared budget is already
|
||||
// spent this window, or the stamp write fails (storage unavailable), we return
|
||||
// without reloading 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" });
|
||||
// the new chunk manifest. Auto-reload at most once per RELOAD_WINDOW_MS: this
|
||||
// recovers across multiple deploys in a single tab's lifetime, yet a
|
||||
// permanently-broken lazy chunk (which would loop) is stopped after the first
|
||||
// reload and falls through to the manual recovery UI below.
|
||||
try {
|
||||
const raw = sessionStorage.getItem(RELOAD_AT_KEY);
|
||||
const lastReloadAt = raw === null ? null : Number.parseInt(raw, 10);
|
||||
const now = Date.now();
|
||||
if (!shouldAutoReload(now, lastReloadAt, RELOAD_WINDOW_MS)) return;
|
||||
sessionStorage.setItem(RELOAD_AT_KEY, String(now));
|
||||
} catch {
|
||||
// sessionStorage unavailable (private mode / disabled): skip the automatic
|
||||
// reload rather than risk an unguarded loop; the fallback UI still recovers.
|
||||
return;
|
||||
}
|
||||
window.location.reload();
|
||||
}
|
||||
|
||||
|
||||
@@ -27,11 +27,15 @@ vi.mock("@/features/space/queries/space-query.ts", () => ({
|
||||
import {
|
||||
buildChildrenByParent,
|
||||
CommentEditorWithActions,
|
||||
sortResolvedByResolvedAt,
|
||||
} from "./comment-list-with-tabs";
|
||||
|
||||
const c = (id: string, parentCommentId: string | null = null): IComment =>
|
||||
({ id, parentCommentId }) as IComment;
|
||||
|
||||
const resolvedAtComment = (id: string, resolvedAt: unknown): IComment =>
|
||||
({ id, resolvedAt }) as unknown as IComment;
|
||||
|
||||
describe("buildChildrenByParent (childrenByParent grouping)", () => {
|
||||
it("returns an empty map for undefined or empty input", () => {
|
||||
expect(buildChildrenByParent(undefined).size).toBe(0);
|
||||
@@ -71,6 +75,48 @@ describe("buildChildrenByParent (childrenByParent grouping)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("sortResolvedByResolvedAt (Resolved tab order, #542)", () => {
|
||||
it("orders by resolvedAt DESC — newest resolve first — with ISO-STRING values", () => {
|
||||
// At runtime resolvedAt is an ISO string (axios JSON / WS subscription), so
|
||||
// the sort must coerce with new Date(...) before .getTime().
|
||||
const older = resolvedAtComment("older", "2026-07-10T10:00:00.000Z");
|
||||
const newest = resolvedAtComment("newest", "2026-07-12T10:00:00.000Z");
|
||||
const middle = resolvedAtComment("middle", "2026-07-11T10:00:00.000Z");
|
||||
|
||||
const out = sortResolvedByResolvedAt([older, newest, middle]);
|
||||
expect(out.map((x) => x.id)).toEqual(["newest", "middle", "older"]);
|
||||
});
|
||||
|
||||
it("also handles Date instances (optimistic onMutate window)", () => {
|
||||
const older = resolvedAtComment("older", new Date("2026-01-01T00:00:00Z"));
|
||||
const newer = resolvedAtComment("newer", new Date("2026-06-01T00:00:00Z"));
|
||||
expect(sortResolvedByResolvedAt([older, newer]).map((x) => x.id)).toEqual([
|
||||
"newer",
|
||||
"older",
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not mutate the input array", () => {
|
||||
const a = resolvedAtComment("a", "2026-01-01T00:00:00.000Z");
|
||||
const b = resolvedAtComment("b", "2026-02-01T00:00:00.000Z");
|
||||
const input = [a, b];
|
||||
sortResolvedByResolvedAt(input);
|
||||
expect(input.map((x) => x.id)).toEqual(["a", "b"]);
|
||||
});
|
||||
|
||||
it("keeps stable order for equal resolvedAt timestamps", () => {
|
||||
const ts = "2026-03-03T03:03:03.000Z";
|
||||
const x = resolvedAtComment("x", ts);
|
||||
const y = resolvedAtComment("y", ts);
|
||||
const z = resolvedAtComment("z", ts);
|
||||
expect(sortResolvedByResolvedAt([x, y, z]).map((c) => c.id)).toEqual([
|
||||
"x",
|
||||
"y",
|
||||
"z",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
function renderReplyEditor() {
|
||||
return render(
|
||||
<MantineProvider>
|
||||
|
||||
@@ -53,6 +53,22 @@ export function buildChildrenByParent(
|
||||
return m;
|
||||
}
|
||||
|
||||
// Sort the Resolved tab by resolve time, newest first, on a COPY (never mutate
|
||||
// the react-query cache array). `resolvedAt` is typed `Date` but at runtime it
|
||||
// is an ISO STRING (from the axios-JSON onSuccess and the WS subscription) — a
|
||||
// real Date only during the optimistic onMutate window — so it MUST be coerced
|
||||
// with `new Date(...)` before `.getTime()`, or a raw `.getTime()` on the string
|
||||
// throws / yields NaN. ES2019's stable sort preserves order for equal
|
||||
// timestamps. Callers pass a list already filtered to a truthy `resolvedAt`, so
|
||||
// the non-null assertion is safe.
|
||||
// Exported for unit testing.
|
||||
export function sortResolvedByResolvedAt(resolved: IComment[]): IComment[] {
|
||||
return [...resolved].sort(
|
||||
(a, b) =>
|
||||
new Date(b.resolvedAt!).getTime() - new Date(a.resolvedAt!).getTime(),
|
||||
);
|
||||
}
|
||||
|
||||
function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
const { t } = useTranslation();
|
||||
const { pageSlug } = useParams();
|
||||
@@ -91,7 +107,10 @@ function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
(comment: IComment) => comment.resolvedAt,
|
||||
);
|
||||
|
||||
return { activeComments: active, resolvedComments: resolved };
|
||||
return {
|
||||
activeComments: active,
|
||||
resolvedComments: sortResolvedByResolvedAt(resolved),
|
||||
};
|
||||
}, [comments]);
|
||||
|
||||
// Index replies by their parent once, instead of an O(n^2) filter per thread.
|
||||
|
||||
@@ -0,0 +1,353 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import React from "react";
|
||||
import { renderHook, waitFor } from "@testing-library/react";
|
||||
import {
|
||||
QueryClient,
|
||||
QueryClientProvider,
|
||||
InfiniteData,
|
||||
} from "@tanstack/react-query";
|
||||
|
||||
/**
|
||||
* Coverage for the resolve/reopen mutation (#542): the Undo-in-toast reopen and
|
||||
* its double-click guard, the terminal 404 branch (drop from cache + clear the
|
||||
* inline mark, no rollback), and the directional error copy.
|
||||
*/
|
||||
|
||||
// A fake TipTap editor injected via the mocked pageEditorAtom, so we can assert
|
||||
// the mutation clears the inline comment mark (unsetComment / setCommentResolved).
|
||||
const editorMock = vi.hoisted(() => ({
|
||||
current: {
|
||||
isDestroyed: false,
|
||||
commands: { unsetComment: vi.fn(), setCommentResolved: vi.fn() },
|
||||
} as {
|
||||
isDestroyed: boolean;
|
||||
commands: {
|
||||
unsetComment: (id: string) => void;
|
||||
setCommentResolved: (id: string, v: boolean) => void;
|
||||
};
|
||||
} | null,
|
||||
}));
|
||||
|
||||
vi.mock("@mantine/notifications", () => ({
|
||||
notifications: { show: vi.fn(), hide: vi.fn() },
|
||||
}));
|
||||
|
||||
vi.mock("jotai", () => ({
|
||||
atom: (v: unknown) => v,
|
||||
useAtomValue: () => editorMock.current,
|
||||
}));
|
||||
|
||||
vi.mock("@/features/comment/services/comment-service", () => ({
|
||||
applySuggestion: vi.fn(),
|
||||
dismissSuggestion: vi.fn(),
|
||||
createComment: vi.fn(),
|
||||
updateComment: vi.fn(),
|
||||
deleteComment: vi.fn(),
|
||||
resolveComment: vi.fn(),
|
||||
getPageComments: vi.fn(),
|
||||
}));
|
||||
|
||||
import { notifications } from "@mantine/notifications";
|
||||
import { resolveComment } from "@/features/comment/services/comment-service";
|
||||
import {
|
||||
useResolveCommentMutation,
|
||||
RESOLVE_UNDO_AUTOCLOSE_MS,
|
||||
RQ_KEY,
|
||||
} from "@/features/comment/queries/comment-query";
|
||||
import { IComment } from "@/features/comment/types/comment.types";
|
||||
|
||||
const PAGE_ID = "page-1";
|
||||
|
||||
function seededClient(comment: IComment) {
|
||||
const queryClient = new QueryClient({
|
||||
defaultOptions: { mutations: { retry: false } },
|
||||
});
|
||||
const seed: InfiniteData<any> = {
|
||||
pageParams: [undefined],
|
||||
pages: [
|
||||
{ items: [comment], meta: { hasNextPage: false, nextCursor: null } },
|
||||
],
|
||||
};
|
||||
queryClient.setQueryData(RQ_KEY(PAGE_ID), seed);
|
||||
const wrapper = ({ children }: { children: React.ReactNode }) => (
|
||||
<QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
|
||||
);
|
||||
return { queryClient, wrapper };
|
||||
}
|
||||
|
||||
function items(queryClient: QueryClient): IComment[] {
|
||||
const cache = queryClient.getQueryData(RQ_KEY(PAGE_ID)) as
|
||||
| InfiniteData<any>
|
||||
| undefined;
|
||||
return cache?.pages.flatMap((p) => p.items) ?? [];
|
||||
}
|
||||
|
||||
const comment = (over?: Partial<IComment>): IComment =>
|
||||
({
|
||||
id: "c-1",
|
||||
pageId: PAGE_ID,
|
||||
content: "{}",
|
||||
creatorId: "u-1",
|
||||
workspaceId: "ws-1",
|
||||
createdAt: new Date(),
|
||||
resolvedAt: null,
|
||||
...over,
|
||||
}) as IComment;
|
||||
|
||||
// Pull the inline Undo button's onClick out of the success toast's message tree.
|
||||
function undoOnClickFromToast(): () => void {
|
||||
const call = vi
|
||||
.mocked(notifications.show)
|
||||
.mock.calls.map((c) => c[0])
|
||||
.find((arg: any) => arg?.autoClose === RESOLVE_UNDO_AUTOCLOSE_MS);
|
||||
expect(call).toBeTruthy();
|
||||
const message: any = (call as any).message;
|
||||
// message = Group( Text, Button ); grab the Button element's onClick.
|
||||
const children = message.props.children as any[];
|
||||
const button = children[1];
|
||||
return button.props.onClick;
|
||||
}
|
||||
|
||||
describe("useResolveCommentMutation — Undo toast (#542)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
editorMock.current = {
|
||||
isDestroyed: false,
|
||||
commands: { unsetComment: vi.fn(), setCommentResolved: vi.fn() },
|
||||
};
|
||||
});
|
||||
|
||||
it("resolve shows an Undo toast with autoClose=10000ms; reopen shows NO Undo", async () => {
|
||||
vi.mocked(resolveComment).mockImplementation(async (data) =>
|
||||
comment({
|
||||
resolvedAt: data.resolved ? (new Date() as any) : null,
|
||||
}),
|
||||
);
|
||||
const { wrapper } = seededClient(comment());
|
||||
const { result } = renderHook(() => useResolveCommentMutation(), {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
await result.current.mutateAsync({
|
||||
commentId: "c-1",
|
||||
pageId: PAGE_ID,
|
||||
resolved: true,
|
||||
});
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
|
||||
const resolveToast = vi
|
||||
.mocked(notifications.show)
|
||||
.mock.calls.map((c) => c[0])
|
||||
.find((a: any) => a?.autoClose === RESOLVE_UNDO_AUTOCLOSE_MS);
|
||||
expect(resolveToast).toBeTruthy();
|
||||
expect((resolveToast as any).id).toBe("resolve-undo-c-1");
|
||||
expect((resolveToast as any).autoClose).toBe(10000);
|
||||
|
||||
// Now a reopen → plain toast, no autoClose/Undo, no id.
|
||||
vi.clearAllMocks();
|
||||
await result.current.mutateAsync({
|
||||
commentId: "c-1",
|
||||
pageId: PAGE_ID,
|
||||
resolved: false,
|
||||
});
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
const calls = vi.mocked(notifications.show).mock.calls.map((c) => c[0]);
|
||||
expect(
|
||||
calls.some((a: any) => a?.autoClose === RESOLVE_UNDO_AUTOCLOSE_MS),
|
||||
).toBe(false);
|
||||
expect(calls).toContainEqual({ message: "Comment re-opened successfully" });
|
||||
});
|
||||
|
||||
it("double/fast Undo click fires reopen EXACTLY once (guard)", async () => {
|
||||
vi.mocked(resolveComment).mockImplementation(async (data) =>
|
||||
comment({ resolvedAt: data.resolved ? (new Date() as any) : null }),
|
||||
);
|
||||
const { wrapper } = seededClient(comment());
|
||||
const { result } = renderHook(() => useResolveCommentMutation(), {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
await result.current.mutateAsync({
|
||||
commentId: "c-1",
|
||||
pageId: PAGE_ID,
|
||||
resolved: true,
|
||||
});
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
|
||||
const onClick = undoOnClickFromToast();
|
||||
// Fire twice synchronously (notifications.hide is not synchronous).
|
||||
onClick();
|
||||
onClick();
|
||||
|
||||
await waitFor(() => {
|
||||
const reopenCalls = vi
|
||||
.mocked(resolveComment)
|
||||
.mock.calls.filter(([d]) => d.resolved === false);
|
||||
expect(reopenCalls).toHaveLength(1);
|
||||
});
|
||||
// The mark was cleared once via setCommentResolved(id, false).
|
||||
expect(editorMock.current!.commands.setCommentResolved).toHaveBeenCalledWith(
|
||||
"c-1",
|
||||
false,
|
||||
);
|
||||
// The toast was hidden.
|
||||
expect(notifications.hide).toHaveBeenCalledWith("resolve-undo-c-1");
|
||||
});
|
||||
|
||||
it("404 → drops the comment from cache, clears the inline mark, no rollback, no Undo", async () => {
|
||||
vi.mocked(resolveComment).mockRejectedValue({ response: { status: 404 } });
|
||||
const { queryClient, wrapper } = seededClient(comment());
|
||||
const { result } = renderHook(() => useResolveCommentMutation(), {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
await result.current
|
||||
.mutateAsync({ commentId: "c-1", pageId: PAGE_ID, resolved: true })
|
||||
.catch(() => undefined);
|
||||
await waitFor(() => expect(result.current.isError).toBe(true));
|
||||
|
||||
// Removed from cache (NOT rolled back to a phantom).
|
||||
expect(items(queryClient)).toHaveLength(0);
|
||||
// Inline mark cleared via unsetComment (mandatory — no panel row left to do it).
|
||||
expect(editorMock.current!.commands.unsetComment).toHaveBeenCalledWith(
|
||||
"c-1",
|
||||
);
|
||||
// Neutral message, red, and crucially NOT the success copy and NO Undo toast.
|
||||
expect(notifications.show).toHaveBeenCalledWith({
|
||||
message: "Comment no longer exists",
|
||||
color: "red",
|
||||
});
|
||||
const calls = vi.mocked(notifications.show).mock.calls.map((c) => c[0]);
|
||||
expect(
|
||||
calls.some((a: any) => a?.autoClose === RESOLVE_UNDO_AUTOCLOSE_MS),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("404 does not crash when the editor is gone (read-only / panel closed)", async () => {
|
||||
editorMock.current = null;
|
||||
vi.mocked(resolveComment).mockRejectedValue({ response: { status: 404 } });
|
||||
const { queryClient, wrapper } = seededClient(comment());
|
||||
const { result } = renderHook(() => useResolveCommentMutation(), {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
await result.current
|
||||
.mutateAsync({ commentId: "c-1", pageId: PAGE_ID, resolved: true })
|
||||
.catch(() => undefined);
|
||||
await waitFor(() => expect(result.current.isError).toBe(true));
|
||||
expect(items(queryClient)).toHaveLength(0);
|
||||
expect(notifications.show).toHaveBeenCalledWith({
|
||||
message: "Comment no longer exists",
|
||||
color: "red",
|
||||
});
|
||||
});
|
||||
|
||||
it("non-404 error on REOPEN shows 'Failed to re-open comment' and rolls back", async () => {
|
||||
vi.mocked(resolveComment).mockRejectedValue({ response: { status: 500 } });
|
||||
// Seed a RESOLVED comment (the reopen target).
|
||||
const resolved = comment({ resolvedAt: new Date() as any });
|
||||
const { queryClient, wrapper } = seededClient(resolved);
|
||||
const { result } = renderHook(() => useResolveCommentMutation(), {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
await result.current
|
||||
.mutateAsync({ commentId: "c-1", pageId: PAGE_ID, resolved: false })
|
||||
.catch(() => undefined);
|
||||
await waitFor(() => expect(result.current.isError).toBe(true));
|
||||
|
||||
expect(notifications.show).toHaveBeenCalledWith({
|
||||
message: "Failed to re-open comment",
|
||||
color: "red",
|
||||
});
|
||||
expect(notifications.show).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({ message: "Failed to resolve comment" }),
|
||||
);
|
||||
// Rolled back: the comment is still present and still resolved.
|
||||
expect(items(queryClient)).toHaveLength(1);
|
||||
expect(items(queryClient)[0].resolvedAt).toBeTruthy();
|
||||
});
|
||||
|
||||
it("reopen via Undo FAILS (non-404) → inline mark is NOT left cleared (doc↔panel stay consistent)", async () => {
|
||||
// First resolve succeeds → produces the Undo toast (no mark change on resolve).
|
||||
vi.mocked(resolveComment).mockResolvedValueOnce(
|
||||
comment({ resolvedAt: new Date() as any }),
|
||||
);
|
||||
const { queryClient, wrapper } = seededClient(comment());
|
||||
const { result } = renderHook(() => useResolveCommentMutation(), {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
await result.current.mutateAsync({
|
||||
commentId: "c-1",
|
||||
pageId: PAGE_ID,
|
||||
resolved: true,
|
||||
});
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
|
||||
// Now the reopen fired by Undo fails with a 500.
|
||||
vi.mocked(resolveComment).mockRejectedValue({ response: { status: 500 } });
|
||||
const onClick = undoOnClickFromToast();
|
||||
onClick();
|
||||
|
||||
await waitFor(() => expect(result.current.isError).toBe(true));
|
||||
|
||||
// Core F1 guarantee: the mark-clear now lives in the reopen onSuccess, so a
|
||||
// FAILED reopen must never flip the inline mark to unresolved — otherwise the
|
||||
// doc would show an active highlight the panel still treats as resolved and
|
||||
// the collab mark would diverge with nothing committed on the server.
|
||||
expect(
|
||||
editorMock.current!.commands.setCommentResolved,
|
||||
).not.toHaveBeenCalledWith("c-1", false);
|
||||
// Cache rolled back: the comment stays resolved and present.
|
||||
expect(items(queryClient)).toHaveLength(1);
|
||||
expect(items(queryClient)[0].resolvedAt).toBeTruthy();
|
||||
});
|
||||
|
||||
it("reopen success with a null editorRef degrades gracefully (no throw, no-op)", async () => {
|
||||
// Read-only view / panel closed: pageEditorAtom is null on the success path.
|
||||
editorMock.current = null;
|
||||
vi.mocked(resolveComment).mockResolvedValue(comment({ resolvedAt: null }));
|
||||
const resolved = comment({ resolvedAt: new Date() as any });
|
||||
const { queryClient, wrapper } = seededClient(resolved);
|
||||
const { result } = renderHook(() => useResolveCommentMutation(), {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
await result.current.mutateAsync({
|
||||
commentId: "c-1",
|
||||
pageId: PAGE_ID,
|
||||
resolved: false,
|
||||
});
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
|
||||
// No crash from the reopen mark-clear; the plain reopen toast is still shown.
|
||||
expect(notifications.show).toHaveBeenCalledWith({
|
||||
message: "Comment re-opened successfully",
|
||||
});
|
||||
// Cache updated to reopened (resolvedAt cleared by the server payload).
|
||||
expect(items(queryClient)).toHaveLength(1);
|
||||
expect(items(queryClient)[0].resolvedAt).toBeFalsy();
|
||||
});
|
||||
|
||||
it("non-404 error on RESOLVE shows 'Failed to resolve comment' and rolls back", async () => {
|
||||
vi.mocked(resolveComment).mockRejectedValue({ response: { status: 500 } });
|
||||
const { queryClient, wrapper } = seededClient(comment());
|
||||
const { result } = renderHook(() => useResolveCommentMutation(), {
|
||||
wrapper,
|
||||
});
|
||||
|
||||
await result.current
|
||||
.mutateAsync({ commentId: "c-1", pageId: PAGE_ID, resolved: true })
|
||||
.catch(() => undefined);
|
||||
await waitFor(() => expect(result.current.isError).toBe(true));
|
||||
|
||||
expect(notifications.show).toHaveBeenCalledWith({
|
||||
message: "Failed to resolve comment",
|
||||
color: "red",
|
||||
});
|
||||
// Rolled back to open (previousCache), still present.
|
||||
expect(items(queryClient)).toHaveLength(1);
|
||||
expect(items(queryClient)[0].resolvedAt).toBeFalsy();
|
||||
});
|
||||
});
|
||||
@@ -20,12 +20,19 @@ import {
|
||||
ISuggestionOutcome,
|
||||
} from "@/features/comment/types/comment.types";
|
||||
import { notifications } from "@mantine/notifications";
|
||||
import { Button, Group, Text } from "@mantine/core";
|
||||
import { IPagination } from "@/lib/types.ts";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useEffect, useMemo } from "react";
|
||||
import React, { useEffect, useMemo, useRef } from "react";
|
||||
import { useAtomValue } from "jotai";
|
||||
import { pageEditorAtom } from "@/features/editor/atoms/editor-atoms";
|
||||
|
||||
export const RQ_KEY = (pageId: string) => ["comments", pageId];
|
||||
|
||||
// How long the resolve success toast (with its inline Undo) stays up before it
|
||||
// auto-closes. Policy constant — no env override.
|
||||
export const RESOLVE_UNDO_AUTOCLOSE_MS = 10000;
|
||||
|
||||
export function useCommentsQuery(params: ICommentParams) {
|
||||
const query = useInfiniteQuery({
|
||||
queryKey: RQ_KEY(params.pageId),
|
||||
@@ -376,7 +383,25 @@ export function useResolveCommentMutation() {
|
||||
const queryClient = useQueryClient();
|
||||
const { t } = useTranslation();
|
||||
|
||||
return useMutation({
|
||||
// Keep the live editor in a ref: the toast's Undo (and the 404 branch) must
|
||||
// clear the inline comment mark AFTER the originating CommentListItem has
|
||||
// unmounted (resolving pulls the comment out of the Open list, so its item is
|
||||
// already gone by the time the 10s toast is clicked). In read-only view
|
||||
// pageEditorAtom is null and the mark converges via the server's
|
||||
// COMMENT_MARK_UPDATE job instead.
|
||||
const editor = useAtomValue(pageEditorAtom);
|
||||
const editorRef = useRef(editor);
|
||||
editorRef.current = editor;
|
||||
|
||||
// Self-reference the mutation so the toast's Undo can re-invoke it (reopen)
|
||||
// long after the triggering component unmounted. Declared BEFORE useMutation
|
||||
// and assigned AFTER; the onClick reads mutationRef.current at CALL time, not
|
||||
// definition time, so there is no initialization cycle.
|
||||
const mutationRef = useRef<{
|
||||
mutate: (vars: IResolveComment) => void;
|
||||
} | null>(null);
|
||||
|
||||
const mutation = useMutation({
|
||||
mutationFn: (data: IResolveComment) => resolveComment(data),
|
||||
onMutate: async (variables) => {
|
||||
await queryClient.cancelQueries({ queryKey: RQ_KEY(variables.pageId) });
|
||||
@@ -401,7 +426,39 @@ export function useResolveCommentMutation() {
|
||||
|
||||
return { previousCache };
|
||||
},
|
||||
onError: (_err, variables, context) => {
|
||||
onError: (err: any, variables, context) => {
|
||||
// Terminal 404: the comment was really deleted (missing comment or deleted
|
||||
// page — access denial is 403, resolve is idempotent so no 400). Do NOT
|
||||
// roll back (that would resurrect a phantom row in Resolved); instead drop
|
||||
// it from the cache and clear its now-orphaned inline mark. Mirrors
|
||||
// handleDeleteComment and the dismiss-mutation 404 branch.
|
||||
if (err?.response?.status === 404) {
|
||||
const cache = queryClient.getQueryData(RQ_KEY(variables.pageId)) as
|
||||
| InfiniteData<IPagination<IComment>>
|
||||
| undefined;
|
||||
if (cache) {
|
||||
queryClient.setQueryData(
|
||||
RQ_KEY(variables.pageId),
|
||||
removeCommentFromCache(cache, variables.commentId),
|
||||
);
|
||||
}
|
||||
const ed = editorRef.current;
|
||||
if (ed && !ed.isDestroyed) {
|
||||
try {
|
||||
ed.commands.unsetComment(variables.commentId);
|
||||
} catch {
|
||||
/* editor gone / mark already removed */
|
||||
}
|
||||
}
|
||||
notifications.show({
|
||||
message: t("Comment no longer exists"),
|
||||
color: "red",
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// Generic failure: roll back the optimistic update and show a DIRECTIONAL
|
||||
// error (resolve vs. reopen), not always "resolve".
|
||||
if (context?.previousCache) {
|
||||
queryClient.setQueryData(
|
||||
RQ_KEY(variables.pageId),
|
||||
@@ -409,7 +466,9 @@ export function useResolveCommentMutation() {
|
||||
);
|
||||
}
|
||||
notifications.show({
|
||||
message: t("Failed to resolve comment"),
|
||||
message: variables.resolved
|
||||
? t("Failed to resolve comment")
|
||||
: t("Failed to re-open comment"),
|
||||
color: "red",
|
||||
});
|
||||
},
|
||||
@@ -430,11 +489,72 @@ export function useResolveCommentMutation() {
|
||||
);
|
||||
}
|
||||
|
||||
// Reopen keeps the plain toast without an Undo.
|
||||
if (!variables.resolved) {
|
||||
// Clear the inline mark ONLY after the server confirms the reopen, so a
|
||||
// failed reopen never leaves an active highlight the panel still treats
|
||||
// as resolved. Mirrors the 404 branch's editor-liveness guard/try-catch.
|
||||
// The button-triggered reopen already set the mark, so this is an
|
||||
// idempotent no-op there.
|
||||
const ed = editorRef.current;
|
||||
if (ed && !ed.isDestroyed) {
|
||||
try {
|
||||
ed.commands.setCommentResolved(variables.commentId, false);
|
||||
} catch {
|
||||
/* editor gone — server COMMENT_MARK_UPDATE converges it */
|
||||
}
|
||||
}
|
||||
notifications.show({ message: t("Comment re-opened successfully") });
|
||||
return;
|
||||
}
|
||||
|
||||
// Resolve: attach an inline Undo (reopen) to the success toast. Built with
|
||||
// React.createElement because this is a .ts module (no JSX).
|
||||
const { commentId, pageId } = variables;
|
||||
const notificationId = `resolve-undo-${commentId}`;
|
||||
// Double-click guard: notifications.hide is NOT synchronous, so the button
|
||||
// stays clickable for a frame or two — without this a fast double-click
|
||||
// would fire reopen twice.
|
||||
let done = false;
|
||||
notifications.show({
|
||||
message: variables.resolved
|
||||
? t("Comment resolved successfully")
|
||||
: t("Comment re-opened successfully"),
|
||||
id: notificationId,
|
||||
autoClose: RESOLVE_UNDO_AUTOCLOSE_MS,
|
||||
message: React.createElement(
|
||||
Group,
|
||||
{ justify: "space-between", wrap: "nowrap", gap: "md" },
|
||||
React.createElement(
|
||||
Text,
|
||||
{ size: "sm" },
|
||||
t("Comment resolved successfully"),
|
||||
),
|
||||
React.createElement(
|
||||
Button,
|
||||
{
|
||||
variant: "subtle",
|
||||
size: "compact-sm",
|
||||
onClick: () => {
|
||||
if (done) return;
|
||||
done = true;
|
||||
// Reopen via the SAME mutation (read at click time — the
|
||||
// originating item is already unmounted).
|
||||
mutationRef.current?.mutate({
|
||||
commentId,
|
||||
pageId,
|
||||
resolved: false,
|
||||
});
|
||||
// The inline mark is cleared in the reopen mutation's onSuccess
|
||||
// (bound to server confirmation), NOT here — clearing it eagerly
|
||||
// would desync the doc from the panel if reopen then fails.
|
||||
notifications.hide(notificationId);
|
||||
},
|
||||
},
|
||||
t("Undo"),
|
||||
),
|
||||
),
|
||||
});
|
||||
},
|
||||
});
|
||||
|
||||
mutationRef.current = mutation;
|
||||
return mutation;
|
||||
}
|
||||
|
||||
@@ -1,195 +0,0 @@
|
||||
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", () => ({
|
||||
notifications: { show: vi.fn() },
|
||||
}));
|
||||
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";
|
||||
|
||||
const show = notifications.show as unknown as ReturnType<typeof vi.fn>;
|
||||
const mockHasAutoReloaded = hasAutoReloaded as unknown as ReturnType<
|
||||
typeof vi.fn
|
||||
>;
|
||||
const mockMarkAutoReloaded = markAutoReloaded as unknown as ReturnType<
|
||||
typeof vi.fn
|
||||
>;
|
||||
|
||||
let reload: ReturnType<typeof vi.fn>;
|
||||
let visibility: DocumentVisibilityState;
|
||||
|
||||
// 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(
|
||||
<MemoryRouter initialEntries={["/start"]}>
|
||||
<Harness />
|
||||
</MemoryRouter>,
|
||||
);
|
||||
}
|
||||
|
||||
function navigateTo(path: string) {
|
||||
act(() => {
|
||||
doNavigate(path);
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
__resetGuardedReloadForTests();
|
||||
vi.clearAllMocks();
|
||||
mockHasAutoReloaded.mockReturnValue(false);
|
||||
mockMarkAutoReloaded.mockReturnValue(true);
|
||||
|
||||
vi.stubGlobal("APP_VERSION", "test-A");
|
||||
|
||||
reload = vi.fn();
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: { reload },
|
||||
});
|
||||
|
||||
visibility = "visible";
|
||||
Object.defineProperty(document, "visibilityState", {
|
||||
configurable: true,
|
||||
get: () => visibility,
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
describe("triggerGuardedReload (variant C)", () => {
|
||||
it("noop when versions match: no banner, no reload", () => {
|
||||
triggerGuardedReload("test-A");
|
||||
expect(show).not.toHaveBeenCalled();
|
||||
expect(reload).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("noop when the server version is empty (fail-safe)", () => {
|
||||
triggerGuardedReload("");
|
||||
triggerGuardedReload(undefined);
|
||||
expect(show).not.toHaveBeenCalled();
|
||||
expect(reload).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("real mismatch shows the banner but does NOT reload immediately", () => {
|
||||
mountHarness();
|
||||
triggerGuardedReload("test-B");
|
||||
expect(show).toHaveBeenCalledTimes(1);
|
||||
expect(show.mock.calls[0][0]).toMatchObject({
|
||||
id: "app-version-reload",
|
||||
autoClose: false,
|
||||
withCloseButton: true,
|
||||
});
|
||||
expect(reload).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
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("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(reload).not.toHaveBeenCalled();
|
||||
|
||||
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);
|
||||
mountHarness();
|
||||
triggerGuardedReload("test-B");
|
||||
navigateTo("/next");
|
||||
expect(reload).not.toHaveBeenCalled();
|
||||
// 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", () => {
|
||||
triggerGuardedReload("test-B");
|
||||
triggerGuardedReload("test-B");
|
||||
triggerGuardedReload("test-C");
|
||||
expect(show).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -1,188 +0,0 @@
|
||||
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,
|
||||
recordReloadBreadcrumb,
|
||||
takeReloadBreadcrumb,
|
||||
} from "@/lib/reload-guard";
|
||||
import { decideVersionAction } from "@/features/user/version-coherence";
|
||||
|
||||
// Dirty shell around the pure `decideVersionAction`: it reads globals
|
||||
// (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 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
|
||||
// pure decision no-op (fail-safe).
|
||||
function readClientVersion(): string {
|
||||
return (typeof APP_VERSION !== "undefined" ? APP_VERSION : "").trim();
|
||||
}
|
||||
|
||||
// Perform the actual reload — but only after the shared one-shot flag is
|
||||
// persisted. If the write fails (storage unavailable) we must NOT reload
|
||||
// (mirrors the reactive chunk-load boundary's `catch → return`), and fall back
|
||||
// to the manual banner so the user can still recover.
|
||||
function performAutoReload(): void {
|
||||
if (!markAutoReloaded()) {
|
||||
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();
|
||||
}
|
||||
|
||||
function showReloadBanner(): void {
|
||||
notifications.show({
|
||||
id: BANNER_ID,
|
||||
title: i18n.t("A new version is available"),
|
||||
message: (
|
||||
<Button size="xs" mt="xs" onClick={() => performAutoReload()}>
|
||||
{i18n.t("Update")}
|
||||
</Button>
|
||||
),
|
||||
autoClose: false,
|
||||
withCloseButton: true,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Handle a server `app-version` announcement: compare it to this bundle's
|
||||
* version and, on a real mismatch, show the banner and arm a guarded reload for
|
||||
* the next in-app navigation (variant C).
|
||||
*
|
||||
* - real mismatch (window budget available) → banner + arm navigation reload.
|
||||
* The banner's "Update" button reloads immediately (same shared window guard).
|
||||
* The tab is NOT reloaded on visibility change.
|
||||
* - auto-reload already used this window / storage error → banner only (no arm),
|
||||
* so there is at most one automatic reload per RELOAD_WINDOW_MS window (loop
|
||||
* safety).
|
||||
* - in sync / unknown version → noop (fail-safe).
|
||||
*/
|
||||
export function triggerGuardedReload(
|
||||
rawServerVersion: string | undefined | null,
|
||||
): void {
|
||||
const serverVersion = (rawServerVersion ?? "").trim();
|
||||
const clientVersion = readClientVersion();
|
||||
|
||||
// A storage read error surfaces as autoReloadUsed=true → fail toward NOT
|
||||
// reloading (banner only).
|
||||
const autoReloadUsed = hasAutoReloaded();
|
||||
|
||||
const action = decideVersionAction({
|
||||
serverVersion,
|
||||
clientVersion,
|
||||
autoReloadUsed,
|
||||
});
|
||||
if (action === "noop") return;
|
||||
|
||||
// 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 the window's
|
||||
// auto-reload budget already spent). Log for diagnosability; show the banner.
|
||||
console.warn(
|
||||
`[version-coherence] server=${serverVersion} client=${clientVersion}: ` +
|
||||
"auto-reload budget already spent this window — showing manual banner",
|
||||
);
|
||||
showReloadBanner();
|
||||
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();
|
||||
pendingNavReload = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 = "";
|
||||
}
|
||||
@@ -1,171 +0,0 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { render, act, cleanup } from "@testing-library/react";
|
||||
import { MemoryRouter, useNavigate } from "react-router-dom";
|
||||
|
||||
// Integration test for the SHARED, window-based auto-reload budget (invariant a):
|
||||
// the reactive chunk-load boundary and the proactive version-coherence path both
|
||||
// route through the REAL @/lib/reload-guard, so at most one automatic reload
|
||||
// happens per RELOAD_WINDOW_MS across BOTH paths combined. Only the two paths'
|
||||
// side-effecting collaborators are mocked — the reload guard is intentionally
|
||||
// REAL so this exercises the actual shared sessionStorage budget.
|
||||
vi.mock("@mantine/notifications", () => ({
|
||||
notifications: { show: vi.fn() },
|
||||
}));
|
||||
vi.mock("@/i18n.ts", () => ({ default: { t: (k: string) => k } }));
|
||||
|
||||
import { handleError } from "@/components/chunk-load-error-boundary";
|
||||
import {
|
||||
triggerGuardedReload,
|
||||
useVersionReloadOnNavigation,
|
||||
__resetGuardedReloadForTests,
|
||||
} from "./guarded-reload";
|
||||
import { RELOAD_WINDOW_MS } from "@/lib/reload-guard";
|
||||
|
||||
const CHUNK_ERROR = { name: "ChunkLoadError", message: "boom" };
|
||||
const T0 = 1_000_000_000_000;
|
||||
|
||||
let reload: ReturnType<typeof vi.fn>;
|
||||
let nowMock: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
// Harness mounted inside a router: installs the navigation hook and exposes
|
||||
// `navigate` so a test can drive an in-app router navigation (the point where the
|
||||
// proactive path fires its armed reload).
|
||||
let doNavigate: (to: string) => void;
|
||||
function Harness() {
|
||||
useVersionReloadOnNavigation();
|
||||
doNavigate = useNavigate();
|
||||
return null;
|
||||
}
|
||||
function mountHarness() {
|
||||
render(
|
||||
<MemoryRouter initialEntries={["/start"]}>
|
||||
<Harness />
|
||||
</MemoryRouter>,
|
||||
);
|
||||
}
|
||||
function navigateTo(path: string) {
|
||||
act(() => {
|
||||
doNavigate(path);
|
||||
});
|
||||
}
|
||||
function setNow(t: number) {
|
||||
nowMock.mockReturnValue(t);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
sessionStorage.clear();
|
||||
__resetGuardedReloadForTests();
|
||||
vi.clearAllMocks();
|
||||
nowMock = vi.spyOn(Date, "now").mockReturnValue(T0);
|
||||
vi.stubGlobal("APP_VERSION", "test-A");
|
||||
reload = vi.fn();
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: { reload },
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.unstubAllGlobals();
|
||||
vi.restoreAllMocks();
|
||||
sessionStorage.clear();
|
||||
});
|
||||
|
||||
describe("shared window-based reload budget (invariant a)", () => {
|
||||
it("a chunk-load reload spends the budget: a version-coherence mismatch within the window shows the banner but does NOT reload", () => {
|
||||
// Path 1 (reactive): a stale-chunk 404 auto-reloads once and stamps the window.
|
||||
handleError(CHUNK_ERROR);
|
||||
expect(reload).toHaveBeenCalledTimes(1);
|
||||
reload.mockClear();
|
||||
|
||||
// Path 2 (proactive), 1 min later — still inside the window. The shared budget
|
||||
// is spent, so the version mismatch degrades to the banner and never reloads.
|
||||
setNow(T0 + 60_000);
|
||||
mountHarness();
|
||||
triggerGuardedReload("test-B");
|
||||
navigateTo("/next");
|
||||
expect(reload).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("a version-coherence reload spends the SAME budget: a chunk-load error within the window does NOT reload", () => {
|
||||
// Path 2 (proactive) first: real mismatch → arm → fire on navigation.
|
||||
mountHarness();
|
||||
triggerGuardedReload("test-B");
|
||||
navigateTo("/next");
|
||||
expect(reload).toHaveBeenCalledTimes(1);
|
||||
reload.mockClear();
|
||||
|
||||
// Path 1 (reactive), 2 min later — inside the window. Budget already spent by
|
||||
// the proactive path, so the stale-chunk error must NOT trigger a second reload.
|
||||
setNow(T0 + 2 * 60_000);
|
||||
handleError(CHUNK_ERROR);
|
||||
expect(reload).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("recovers after the window: a reload strictly older than the window is allowed again (second deploy)", () => {
|
||||
// First auto-reload (reactive) stamps the window.
|
||||
handleError(CHUNK_ERROR);
|
||||
expect(reload).toHaveBeenCalledTimes(1);
|
||||
reload.mockClear();
|
||||
|
||||
// A second deploy arrives after the window has fully elapsed → the proactive
|
||||
// path is allowed to reload again (window, not a permanent one-shot).
|
||||
__resetGuardedReloadForTests();
|
||||
setNow(T0 + RELOAD_WINDOW_MS + 1);
|
||||
mountHarness();
|
||||
triggerGuardedReload("test-B");
|
||||
navigateTo("/next");
|
||||
expect(reload).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("reactive path fails closed when storage READS but cannot WRITE (quota / Safari private): no unguarded reload", () => {
|
||||
// getItem→null makes hasAutoReloaded() report the budget as available, so
|
||||
// handleError passes the first guard and reaches `if (!markAutoReloaded())
|
||||
// return;`. setItem throws → the stamp cannot stick, so markAutoReloaded()
|
||||
// returns false and that guard MUST bail — otherwise the reactive path would
|
||||
// reload on every stale-chunk error with no persisted budget (an unguarded
|
||||
// loop). This is the asymmetric gap: the proactive path's equivalent is
|
||||
// covered by guarded-reload.test.tsx "does NOT reload when the flag write
|
||||
// fails".
|
||||
vi.stubGlobal("sessionStorage", {
|
||||
getItem: () => null,
|
||||
setItem: () => {
|
||||
throw new Error("quota exceeded");
|
||||
},
|
||||
removeItem: () => {},
|
||||
clear: () => {},
|
||||
});
|
||||
try {
|
||||
handleError(CHUNK_ERROR);
|
||||
expect(reload).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
vi.unstubAllGlobals();
|
||||
}
|
||||
});
|
||||
|
||||
it("sessionStorage unavailable: neither path performs an unguarded reload", () => {
|
||||
// The real guard fails toward NOT reloading when storage throws.
|
||||
vi.stubGlobal("sessionStorage", {
|
||||
getItem: () => {
|
||||
throw new Error("storage disabled");
|
||||
},
|
||||
setItem: () => {
|
||||
throw new Error("storage disabled");
|
||||
},
|
||||
removeItem: () => {},
|
||||
clear: () => {},
|
||||
});
|
||||
try {
|
||||
handleError(CHUNK_ERROR);
|
||||
expect(reload).not.toHaveBeenCalled();
|
||||
|
||||
mountHarness();
|
||||
triggerGuardedReload("test-B");
|
||||
navigateTo("/next");
|
||||
expect(reload).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
vi.unstubAllGlobals();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -13,12 +13,6 @@ 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,
|
||||
useVersionReloadOnNavigation,
|
||||
surfacePreviousReloadBreadcrumb,
|
||||
} from "@/features/user/guarded-reload.tsx";
|
||||
import type { AppVersionSocketPayload } from "@/features/user/version-coherence.ts";
|
||||
|
||||
export function UserProvider({ children }: React.PropsWithChildren) {
|
||||
const [, setCurrentUser] = useAtom(currentUserAtom);
|
||||
@@ -28,16 +22,6 @@ 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;
|
||||
@@ -63,16 +47,6 @@ export function UserProvider({ children }: React.PropsWithChildren) {
|
||||
handleConnect();
|
||||
});
|
||||
|
||||
// Register the version-coherence listener SYNCHRONOUSLY, before the socket
|
||||
// connects: the server emits `app-version` immediately in handleConnection,
|
||||
// so a listener attached after connect would miss it on a fast localhost
|
||||
// connect. On a version mismatch the client shows a banner and defers the
|
||||
// auto-reload to the next in-app navigation (variant C — avoids reloading a
|
||||
// backgrounded tab that may hold unsaved input) before it hits a stale chunk.
|
||||
newSocket.on("app-version", (payload?: AppVersionSocketPayload) => {
|
||||
triggerGuardedReload(payload?.version);
|
||||
});
|
||||
|
||||
return () => {
|
||||
console.log("ws disconnected");
|
||||
newSocket.disconnect();
|
||||
|
||||
@@ -1,64 +0,0 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { decideVersionAction } from "./version-coherence";
|
||||
|
||||
describe("decideVersionAction", () => {
|
||||
it("noop when the server version is empty (fail-safe)", () => {
|
||||
expect(
|
||||
decideVersionAction({
|
||||
serverVersion: "",
|
||||
clientVersion: "v1",
|
||||
autoReloadUsed: false,
|
||||
}),
|
||||
).toBe("noop");
|
||||
});
|
||||
|
||||
it("noop when the client version is empty (fail-safe)", () => {
|
||||
expect(
|
||||
decideVersionAction({
|
||||
serverVersion: "v1",
|
||||
clientVersion: "",
|
||||
autoReloadUsed: false,
|
||||
}),
|
||||
).toBe("noop");
|
||||
});
|
||||
|
||||
it("noop when versions are equal (in sync)", () => {
|
||||
expect(
|
||||
decideVersionAction({
|
||||
serverVersion: "v1",
|
||||
clientVersion: "v1",
|
||||
autoReloadUsed: false,
|
||||
}),
|
||||
).toBe("noop");
|
||||
});
|
||||
|
||||
it("reload on a real mismatch the first time this session", () => {
|
||||
expect(
|
||||
decideVersionAction({
|
||||
serverVersion: "test-B",
|
||||
clientVersion: "test-A",
|
||||
autoReloadUsed: false,
|
||||
}),
|
||||
).toBe("reload");
|
||||
});
|
||||
|
||||
it("banner on a mismatch once the session auto-reload is spent", () => {
|
||||
expect(
|
||||
decideVersionAction({
|
||||
serverVersion: "test-B",
|
||||
clientVersion: "test-A",
|
||||
autoReloadUsed: true,
|
||||
}),
|
||||
).toBe("banner");
|
||||
});
|
||||
|
||||
it("equal versions stay noop even if auto-reload was already used", () => {
|
||||
expect(
|
||||
decideVersionAction({
|
||||
serverVersion: "v1",
|
||||
clientVersion: "v1",
|
||||
autoReloadUsed: true,
|
||||
}),
|
||||
).toBe("noop");
|
||||
});
|
||||
});
|
||||
@@ -1,32 +0,0 @@
|
||||
// Payload of the per-connect `app-version` socket.io event announced by the
|
||||
// server (ws.gateway.ts) after a successful auth. A dedicated event — NOT a
|
||||
// member of the room-scoped `WebSocketEvent` union (which is discriminated by
|
||||
// `operation`), so it never touches use-query-subscription.
|
||||
export type AppVersionSocketPayload = { version: string };
|
||||
|
||||
/**
|
||||
* Pure decision for the version-coherence guard.
|
||||
*
|
||||
* All inputs are injected (no globals, no side effects) so it is unit-testable
|
||||
* without a DOM or the build-time `APP_VERSION` global (undefined under vitest).
|
||||
*
|
||||
* - `autoReloadUsed` = an automatic reload has already happened within the
|
||||
* current ~5-min window, so we must not auto-reload again (loop safety,
|
||||
* shared window budget with the reactive chunk-load boundary).
|
||||
*
|
||||
* Returns:
|
||||
* - "noop" — do nothing (unknown version on either side, or already in sync).
|
||||
* - "banner" — show the manual "update available" banner only (no auto-reload).
|
||||
* - "reload" — real first-time mismatch: eligible for a guarded auto-reload.
|
||||
*/
|
||||
export function decideVersionAction(args: {
|
||||
serverVersion: string;
|
||||
clientVersion: string;
|
||||
autoReloadUsed: boolean;
|
||||
}): "reload" | "banner" | "noop" {
|
||||
const { serverVersion, clientVersion, autoReloadUsed } = args;
|
||||
if (!serverVersion || !clientVersion) return "noop"; // fail-safe: unknown version → never act
|
||||
if (serverVersion === clientVersion) return "noop"; // in sync
|
||||
if (autoReloadUsed) return "banner"; // one auto-reload per RELOAD_WINDOW_MS window already spent
|
||||
return "reload"; // real mismatch, window budget available
|
||||
}
|
||||
@@ -1,145 +0,0 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import {
|
||||
hasAutoReloaded,
|
||||
markAutoReloaded,
|
||||
shouldAutoReload,
|
||||
recordReloadBreadcrumb,
|
||||
takeReloadBreadcrumb,
|
||||
RELOAD_WINDOW_MS,
|
||||
} from "./reload-guard";
|
||||
|
||||
// The shared budget is a single sessionStorage timestamp keyed here; both the
|
||||
// reactive chunk-load boundary and the proactive version-coherence path read and
|
||||
// stamp it, so at most one auto-reload happens per RELOAD_WINDOW_MS across BOTH.
|
||||
const RELOAD_AT_KEY = "chunk-reload-at";
|
||||
const NOW = 1_000_000_000_000;
|
||||
|
||||
describe("reload-guard", () => {
|
||||
beforeEach(() => {
|
||||
sessionStorage.clear();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
sessionStorage.clear();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("hasAutoReloaded is false before any reload, true within the window after mark", () => {
|
||||
expect(hasAutoReloaded(NOW)).toBe(false);
|
||||
expect(markAutoReloaded(NOW)).toBe(true);
|
||||
// Same key both paths share; stores the reload timestamp, not a flag.
|
||||
expect(sessionStorage.getItem(RELOAD_AT_KEY)).toBe(String(NOW));
|
||||
// Inside the window → budget spent → true (fall through to manual UI).
|
||||
expect(hasAutoReloaded(NOW)).toBe(true);
|
||||
expect(hasAutoReloaded(NOW + RELOAD_WINDOW_MS)).toBe(true);
|
||||
});
|
||||
|
||||
it("hasAutoReloaded is false again once the window has elapsed (a later deploy recovers)", () => {
|
||||
markAutoReloaded(NOW);
|
||||
// Strictly older than the window → a new deploy's mismatch may reload again.
|
||||
expect(hasAutoReloaded(NOW + RELOAD_WINDOW_MS + 1)).toBe(false);
|
||||
});
|
||||
|
||||
it("hasAutoReloaded returns true when reading storage throws (fail toward not reloading)", () => {
|
||||
vi.stubGlobal("sessionStorage", {
|
||||
getItem: () => {
|
||||
throw new Error("storage disabled");
|
||||
},
|
||||
setItem: () => {
|
||||
throw new Error("storage disabled");
|
||||
},
|
||||
});
|
||||
try {
|
||||
expect(hasAutoReloaded()).toBe(true);
|
||||
} finally {
|
||||
vi.unstubAllGlobals();
|
||||
}
|
||||
});
|
||||
|
||||
it("hasAutoReloaded treats an unparseable stored timestamp as never-reloaded", () => {
|
||||
sessionStorage.setItem(RELOAD_AT_KEY, "not-a-number");
|
||||
expect(hasAutoReloaded(NOW)).toBe(false);
|
||||
});
|
||||
|
||||
it("markAutoReloaded returns false when writing storage throws", () => {
|
||||
vi.stubGlobal("sessionStorage", {
|
||||
getItem: () => null,
|
||||
setItem: () => {
|
||||
throw new Error("storage disabled");
|
||||
},
|
||||
});
|
||||
try {
|
||||
expect(markAutoReloaded()).toBe(false);
|
||||
} finally {
|
||||
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();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// The pure window gate replaces the old one-shot flag: it must permit recovery
|
||||
// across several deploys in one tab (each > window apart) while still stopping an
|
||||
// infinite reload loop when a lazy chunk is permanently broken (a second failure
|
||||
// < window). Moved here from the chunk-load boundary now that it is the shared
|
||||
// guard both paths route through.
|
||||
describe("shouldAutoReload", () => {
|
||||
const WINDOW = RELOAD_WINDOW_MS;
|
||||
|
||||
it("allows a reload when we have never auto-reloaded", () => {
|
||||
expect(shouldAutoReload(NOW, null, WINDOW)).toBe(true);
|
||||
});
|
||||
|
||||
it("allows a reload when the last one was 6 minutes ago (outside the window)", () => {
|
||||
expect(shouldAutoReload(NOW, NOW - 6 * 60 * 1000, WINDOW)).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks a reload when the last one was 1 minute ago (inside the window)", () => {
|
||||
expect(shouldAutoReload(NOW, NOW - 1 * 60 * 1000, WINDOW)).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks a reload exactly at the window boundary (not strictly older)", () => {
|
||||
expect(shouldAutoReload(NOW, NOW - WINDOW, WINDOW)).toBe(false);
|
||||
});
|
||||
|
||||
it("allows a reload when the stored timestamp is unparseable (NaN)", () => {
|
||||
expect(shouldAutoReload(NOW, NaN, WINDOW)).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -1,121 +0,0 @@
|
||||
// Shared, window-based auto-reload budget.
|
||||
//
|
||||
// Both auto-reload paths — the reactive chunk-load-error-boundary (recovers
|
||||
// AFTER a stale lazy chunk 404s) and the proactive version-coherence feature
|
||||
// (reloads BEFORE the tab hits a stale chunk) — go through these functions so
|
||||
// they share ONE window-scoped reload budget: at most a single automatic
|
||||
// reload per RELOAD_WINDOW_MS across BOTH paths. A window (rather than a
|
||||
// permanent one-shot flag) lets a SECOND deploy in the same tab's lifetime
|
||||
// recover too, while a permanent skew, node oscillation, or a genuinely-missing
|
||||
// chunk still degrades to a manual banner/UI after the first reload instead of
|
||||
// looping. When sessionStorage is unavailable every mismatch degrades to the
|
||||
// manual UI — no unguarded reload.
|
||||
|
||||
// sessionStorage key holding the epoch-ms timestamp of the last automatic reload
|
||||
// (shared by both paths).
|
||||
const RELOAD_AT_KEY = "chunk-reload-at";
|
||||
|
||||
// Allow at most one automatic reload per this window. A stale-deploy 404 is cured
|
||||
// by a single reload, so anything inside the window is treated as a reload loop
|
||||
// (permanently-broken chunk / permanent skew) and falls through to the manual UI.
|
||||
export const RELOAD_WINDOW_MS = 5 * 60 * 1000;
|
||||
|
||||
/**
|
||||
* Pure window decision, unit-tested in isolation: auto-reload only if we have
|
||||
* never auto-reloaded (lastReloadAt null/NaN) or the last one was strictly older
|
||||
* than the window. Anything inside the window is suppressed to break an infinite
|
||||
* reload loop.
|
||||
*/
|
||||
export function shouldAutoReload(
|
||||
now: number,
|
||||
lastReloadAt: number | null,
|
||||
windowMs: number,
|
||||
): boolean {
|
||||
if (lastReloadAt === null || Number.isNaN(lastReloadAt)) return true;
|
||||
return now - lastReloadAt > windowMs;
|
||||
}
|
||||
|
||||
/**
|
||||
* Has an automatic reload already happened within the current window (so the
|
||||
* shared budget is spent right now)? Both paths check this before reloading; a
|
||||
* `true` return means fall through to the manual banner/UI instead of reloading.
|
||||
*
|
||||
* A storage read error (private mode / disabled) is reported as `true` so the
|
||||
* caller fails toward NOT reloading — an unguarded loop is worse than a stale
|
||||
* tab the user can reload manually. Note a window (not a permanent flag): once
|
||||
* the window elapses a later deploy's mismatch is allowed to reload again.
|
||||
*/
|
||||
export function hasAutoReloaded(now: number = Date.now()): boolean {
|
||||
try {
|
||||
const raw = sessionStorage.getItem(RELOAD_AT_KEY);
|
||||
const lastReloadAt = raw === null ? null : Number.parseInt(raw, 10);
|
||||
return !shouldAutoReload(now, lastReloadAt, RELOAD_WINDOW_MS);
|
||||
} catch {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Stamp the shared window as consumed now — record that an automatic reload is
|
||||
* being performed within the current RELOAD_WINDOW_MS window.
|
||||
*
|
||||
* Returns whether the write succeeded. A `false` return (storage unavailable)
|
||||
* means the caller MUST NOT reload — otherwise the stamp would never stick and
|
||||
* the reload could loop.
|
||||
*/
|
||||
export function markAutoReloaded(now: number = Date.now()): boolean {
|
||||
try {
|
||||
sessionStorage.setItem(RELOAD_AT_KEY, String(now));
|
||||
return true;
|
||||
} catch {
|
||||
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<ReloadBreadcrumb, "at">,
|
||||
): 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;
|
||||
}
|
||||
}
|
||||
@@ -1,8 +1,7 @@
|
||||
import { defineConfig, loadEnv, type Plugin } from "vite";
|
||||
import { defineConfig, loadEnv } from "vite";
|
||||
import react from "@vitejs/plugin-react";
|
||||
import { compression } from "vite-plugin-compression2";
|
||||
import * as path from "path";
|
||||
import * as fs from "node:fs";
|
||||
import { execSync } from "node:child_process";
|
||||
|
||||
const envPath = path.resolve(process.cwd(), "..", "..");
|
||||
@@ -25,32 +24,7 @@ function resolveAppVersion(cwd: string): string {
|
||||
}
|
||||
}
|
||||
|
||||
// Emit <outDir>/version.json = { "version": appVersion } so the server can read
|
||||
// the exact same build id the bundle was compiled with. The value is the SAME
|
||||
// `appVersion` fed into `define.APP_VERSION`, so version.json and the baked-in
|
||||
// global are identical by construction — the single source of truth (no
|
||||
// runtime-env second copy that could drift and cause a false version mismatch).
|
||||
function versionJsonPlugin(version: string): Plugin {
|
||||
let outDir = "dist";
|
||||
return {
|
||||
name: "emit-version-json",
|
||||
apply: "build",
|
||||
configResolved(config) {
|
||||
outDir = config.build.outDir;
|
||||
},
|
||||
writeBundle() {
|
||||
const root = path.resolve(process.cwd(), outDir);
|
||||
fs.mkdirSync(root, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(root, "version.json"),
|
||||
JSON.stringify({ version }),
|
||||
);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export default defineConfig(({ mode }) => {
|
||||
const appVersion = resolveAppVersion(envPath);
|
||||
const {
|
||||
APP_URL,
|
||||
FILE_UPLOAD_SIZE_LIMIT,
|
||||
@@ -78,11 +52,10 @@ export default defineConfig(({ mode }) => {
|
||||
POSTHOG_HOST,
|
||||
POSTHOG_KEY,
|
||||
},
|
||||
APP_VERSION: JSON.stringify(appVersion),
|
||||
APP_VERSION: JSON.stringify(resolveAppVersion(envPath)),
|
||||
},
|
||||
plugins: [
|
||||
react(),
|
||||
versionJsonPlugin(appVersion),
|
||||
// Emit .br and .gz next to every built asset so the server can serve the
|
||||
// precompressed copy (see @fastify/static preCompressed in static.module.ts).
|
||||
compression({
|
||||
|
||||
@@ -1,52 +0,0 @@
|
||||
import { join } from 'path';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import { readClientBuildVersion } from './client-version';
|
||||
|
||||
describe('readClientBuildVersion', () => {
|
||||
let dir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
dir = fs.mkdtempSync(join(os.tmpdir(), 'client-version-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const writeVersionJson = (content: string) =>
|
||||
fs.writeFileSync(join(dir, 'version.json'), content);
|
||||
|
||||
it('returns the version from a valid version.json', () => {
|
||||
writeVersionJson(JSON.stringify({ version: 'test-A' }));
|
||||
expect(readClientBuildVersion(dir)).toBe('test-A');
|
||||
});
|
||||
|
||||
it('trims surrounding whitespace in the version', () => {
|
||||
writeVersionJson(JSON.stringify({ version: ' v1.2.3 ' }));
|
||||
expect(readClientBuildVersion(dir)).toBe('v1.2.3');
|
||||
});
|
||||
|
||||
it('returns "" when version.json is missing', () => {
|
||||
expect(readClientBuildVersion(dir)).toBe('');
|
||||
});
|
||||
|
||||
it('returns "" on malformed JSON', () => {
|
||||
writeVersionJson('{ not json');
|
||||
expect(readClientBuildVersion(dir)).toBe('');
|
||||
});
|
||||
|
||||
it('returns "" when the version field is absent', () => {
|
||||
writeVersionJson(JSON.stringify({ notVersion: 'x' }));
|
||||
expect(readClientBuildVersion(dir)).toBe('');
|
||||
});
|
||||
|
||||
it('returns "" when the version field is not a string', () => {
|
||||
writeVersionJson(JSON.stringify({ version: 123 }));
|
||||
expect(readClientBuildVersion(dir)).toBe('');
|
||||
});
|
||||
|
||||
it('returns "" when the path does not exist at all', () => {
|
||||
expect(readClientBuildVersion(join(dir, 'nope'))).toBe('');
|
||||
});
|
||||
});
|
||||
@@ -1,36 +0,0 @@
|
||||
import { join } from 'path';
|
||||
import * as fs from 'node:fs';
|
||||
|
||||
/**
|
||||
* Resolve the absolute path to the built client bundle directory
|
||||
* (`apps/client/dist`) shipped into the runtime image.
|
||||
*
|
||||
* The `../` depth is anchored on THIS module's compiled location
|
||||
* (`dist/common/helpers`). `integrations/static` sits at the same depth under
|
||||
* the compiled root, so both callers (StaticModule and readClientBuildVersion)
|
||||
* MUST share this single helper rather than duplicating the depth — a copy in a
|
||||
* module at a different depth would silently resolve to the wrong directory.
|
||||
*/
|
||||
export function resolveClientDistPath(): string {
|
||||
return join(__dirname, '..', '..', '..', '..', 'client/dist');
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the build version the client bundle was compiled with, from
|
||||
* `<clientDistPath>/version.json` (written by the Vite build — the single
|
||||
* source of truth shared by the baked-in `APP_VERSION` global and this file).
|
||||
*
|
||||
* Fail-safe: any error (missing file, unreadable, bad JSON, non-string
|
||||
* version) yields `''`. The caller treats an empty version as "unknown" and
|
||||
* the whole version-coherence feature stays silently inert — existing deploys
|
||||
* without the file keep working unchanged.
|
||||
*/
|
||||
export function readClientBuildVersion(clientDistPath: string): string {
|
||||
try {
|
||||
const raw = fs.readFileSync(join(clientDistPath, 'version.json'), 'utf8');
|
||||
const version = (JSON.parse(raw) as { version?: unknown }).version;
|
||||
return typeof version === 'string' ? version.trim() : '';
|
||||
} catch {
|
||||
return '';
|
||||
}
|
||||
}
|
||||
@@ -3,4 +3,3 @@ export * from './nanoid.utils';
|
||||
export * from './file.helper';
|
||||
export * from './constants';
|
||||
export * from './security-headers';
|
||||
export * from './client-version';
|
||||
|
||||
@@ -4,7 +4,6 @@ import { join } from 'path';
|
||||
import * as fs from 'node:fs';
|
||||
import fastifyStatic from '@fastify/static';
|
||||
import { EnvironmentService } from '../environment/environment.service';
|
||||
import { resolveClientDistPath } from '../../common/helpers/client-version';
|
||||
|
||||
/**
|
||||
* Resolve the response headers for a statically served client asset.
|
||||
@@ -57,7 +56,14 @@ export class StaticModule implements OnModuleInit {
|
||||
const httpAdapter = this.httpAdapterHost.httpAdapter;
|
||||
const app = httpAdapter.getInstance();
|
||||
|
||||
const clientDistPath = resolveClientDistPath();
|
||||
const clientDistPath = join(
|
||||
__dirname,
|
||||
'..',
|
||||
'..',
|
||||
'..',
|
||||
'..',
|
||||
'client/dist',
|
||||
);
|
||||
|
||||
const indexFilePath = join(clientDistPath, 'index.html');
|
||||
|
||||
|
||||
@@ -9,14 +9,10 @@ import {
|
||||
import { Server, Socket } from 'socket.io';
|
||||
import { TokenService } from '../core/auth/services/token.service';
|
||||
import { JwtPayload, JwtType } from '../core/auth/dto/jwt-payload';
|
||||
import { Logger, OnModuleDestroy, OnModuleInit } from '@nestjs/common';
|
||||
import { OnModuleDestroy } from '@nestjs/common';
|
||||
import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo';
|
||||
import { WsService } from './ws.service';
|
||||
import { getSpaceRoomName, getUserRoomName } from './ws.utils';
|
||||
import {
|
||||
readClientBuildVersion,
|
||||
resolveClientDistPath,
|
||||
} from '../common/helpers/client-version';
|
||||
import * as cookie from 'cookie';
|
||||
|
||||
@WebSocketGateway({
|
||||
@@ -24,40 +20,17 @@ import * as cookie from 'cookie';
|
||||
transports: ['websocket'],
|
||||
})
|
||||
export class WsGateway
|
||||
implements
|
||||
OnGatewayConnection,
|
||||
OnGatewayInit,
|
||||
OnModuleInit,
|
||||
OnModuleDestroy
|
||||
implements OnGatewayConnection, OnGatewayInit, OnModuleDestroy
|
||||
{
|
||||
@WebSocketServer()
|
||||
server: Server;
|
||||
|
||||
private readonly logger = new Logger(WsGateway.name);
|
||||
|
||||
// The build version of the client bundle shipped in this image, read once at
|
||||
// startup from client/dist/version.json (single source of truth, same value
|
||||
// baked into the client's APP_VERSION). Empty string => version.json missing
|
||||
// or empty => the proactive version-coherence reload feature stays inert.
|
||||
private appVersion = '';
|
||||
|
||||
constructor(
|
||||
private tokenService: TokenService,
|
||||
private spaceMemberRepo: SpaceMemberRepo,
|
||||
private wsService: WsService,
|
||||
) {}
|
||||
|
||||
onModuleInit(): void {
|
||||
this.appVersion = readClientBuildVersion(resolveClientDistPath());
|
||||
if (this.appVersion) {
|
||||
this.logger.log(`app-version reload: ACTIVE (v=${this.appVersion})`);
|
||||
} else {
|
||||
this.logger.log(
|
||||
'app-version reload: DISABLED (version.json missing/empty)',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
afterInit(server: Server): void {
|
||||
this.wsService.setServer(server);
|
||||
}
|
||||
@@ -82,14 +55,6 @@ export class WsGateway
|
||||
const spaceRooms = userSpaceIds.map((id) => getSpaceRoomName(id));
|
||||
|
||||
client.join([userRoom, workspaceRoom, ...spaceRooms]);
|
||||
|
||||
// Announce this container's client build version to the freshly
|
||||
// authenticated socket. On a redeploy the client reconnects to the new
|
||||
// container and receives the new version here, letting it guard-reload
|
||||
// before it hits a stale lazy chunk. Per-connect only (no broadcast):
|
||||
// natural reconnect covers both single-container and cluster without a
|
||||
// thundering-herd fleet reload.
|
||||
client.emit('app-version', { version: this.appVersion });
|
||||
} catch (err) {
|
||||
client.emit('Unauthorized');
|
||||
client.disconnect();
|
||||
|
||||
Reference in New Issue
Block a user