Compare commits
19 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 6bfb1e645a | |||
| 6e59793643 | |||
| 1bcc96685e | |||
| ee33a293b9 | |||
| 86830b860d | |||
| d0d2a7880f | |||
| 9acbc07f7d | |||
| a0eb3131a6 | |||
| 50bb086edf | |||
| f2ad0121a5 | |||
| 2194f423a1 | |||
| 5a6009c750 | |||
| 9685074237 | |||
| 22f687c39e | |||
| 72c2d1687e | |||
| 96faa28220 | |||
| c9293e316b | |||
| 654ba9f249 | |||
| 9120ad3b2d |
@@ -67,14 +67,20 @@ export default function GlobalAppShell({
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
//https://codesandbox.io/p/sandbox/kz9de
|
||||
// Attach the global mousemove/mouseup only WHILE resizing (started on the
|
||||
// handle's mousedown via startResizing → isResizing=true) and detach on
|
||||
// mouseup (stopResizing → isResizing=false). Previously these listeners were
|
||||
// attached for the whole app lifetime, so every mouse move over the app ran
|
||||
// the resize handler.
|
||||
// https://codesandbox.io/p/sandbox/kz9de
|
||||
if (!isResizing) return;
|
||||
window.addEventListener("mousemove", resize);
|
||||
window.addEventListener("mouseup", stopResizing);
|
||||
return () => {
|
||||
window.removeEventListener("mousemove", resize);
|
||||
window.removeEventListener("mouseup", stopResizing);
|
||||
};
|
||||
}, [resize, stopResizing]);
|
||||
}, [isResizing, resize, stopResizing]);
|
||||
|
||||
const location = useLocation();
|
||||
const isSettingsRoute = location.pathname.startsWith("/settings");
|
||||
|
||||
@@ -5,7 +5,7 @@ import {
|
||||
Button,
|
||||
useMantineColorScheme,
|
||||
} from "@mantine/core";
|
||||
import { useClickOutside, useDisclosure, useWindowEvent } from "@mantine/hooks";
|
||||
import { useClickOutside, useDisclosure } from "@mantine/hooks";
|
||||
import { Suspense } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
|
||||
@@ -57,14 +57,22 @@ function EmojiPicker({
|
||||
[dropdown, target],
|
||||
);
|
||||
|
||||
// We need this because the default Mantine popover closeOnEscape does not work
|
||||
useWindowEvent("keydown", (event) => {
|
||||
if (opened && event.key === "Escape") {
|
||||
event.stopPropagation();
|
||||
event.preventDefault();
|
||||
handlers.close();
|
||||
}
|
||||
});
|
||||
// We need this because the default Mantine popover closeOnEscape does not work.
|
||||
// Attach the global keydown ONLY while the picker is open (every tree row
|
||||
// renders an EmojiPicker, so an always-on window listener meant ~20-30 idle
|
||||
// keydown handlers firing on each keystroke).
|
||||
useEffect(() => {
|
||||
if (!opened) return;
|
||||
const handleKeydown = (event: KeyboardEvent) => {
|
||||
if (event.key === "Escape") {
|
||||
event.stopPropagation();
|
||||
event.preventDefault();
|
||||
handlers.close();
|
||||
}
|
||||
};
|
||||
window.addEventListener("keydown", handleKeydown);
|
||||
return () => window.removeEventListener("keydown", handleKeydown);
|
||||
}, [opened, handlers]);
|
||||
|
||||
// emoji-mart's built-in autoFocus calls .focus() without preventScroll, which
|
||||
// makes the browser scroll every scrollable ancestor of the search input to
|
||||
|
||||
@@ -36,7 +36,7 @@ import {
|
||||
desktopSidebarAtom,
|
||||
mobileSidebarAtom,
|
||||
} from "@/components/layouts/global/hooks/atoms/sidebar-atom.ts";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import {
|
||||
pageEditorAtom,
|
||||
readOnlyEditorAtom,
|
||||
@@ -86,11 +86,19 @@ const MIN_HEIGHT = 400;
|
||||
// Margin kept between the window and the viewport edges while dragging.
|
||||
const EDGE_MARGIN = 8;
|
||||
|
||||
// #184 phase 1.5: hard cap on the degraded-poll fallback. The poll is armed when
|
||||
// a resume attempt could not attach to the live run and disarmed by the thread on
|
||||
// settle / local stream; this cap is the ONLY backstop against an endless tick
|
||||
// (a stuck 'streaming' row before the boot-sweep, or a user-tail 204 with no run).
|
||||
const DEGRADED_POLL_MAX_MS = 10 * 60_000;
|
||||
// #184 phase 1.5 / #430: backstop for the degraded-poll fallback. The poll is
|
||||
// armed when a resume attempt could not attach to the live run and disarmed by the
|
||||
// thread on settle / local stream; this cap is the ONLY backstop against an endless
|
||||
// tick (a stuck 'streaming' row before the boot-sweep, or a user-tail 204 with no
|
||||
// run).
|
||||
//
|
||||
// #430: measured from RUN ACTIVITY, not from arm-time. A real autonomous run takes
|
||||
// 11-25 min — longer than a fixed 10-min-from-start cap, which used to cut the poll
|
||||
// off mid-run. Instead we cap on INACTIVITY: keep polling as long as the run is
|
||||
// still making progress (its persisted rows keep changing), and only give up after
|
||||
// this long with NO new activity. A genuinely stuck run produces no row changes, so
|
||||
// the idle cap still bounds it; a long-but-progressing run polls to completion.
|
||||
const DEGRADED_POLL_IDLE_MAX_MS = 10 * 60_000;
|
||||
|
||||
/** Compact token formatter: 1.2M / 3.4k / 950. */
|
||||
function formatTokens(n: number): string {
|
||||
@@ -237,7 +245,9 @@ export default function AiChatWindow() {
|
||||
// left partly off-screen).
|
||||
const [geom, setGeom] = useAtom(aiChatWindowGeomAtom);
|
||||
|
||||
const { data: chats } = useAiChatsQuery();
|
||||
// Gated on windowOpen: the chat list is only needed once the window is open,
|
||||
// so a closed window issues no chat-list request/refetch on navigation.
|
||||
const { data: chats } = useAiChatsQuery(windowOpen);
|
||||
// Roles for the new-chat picker (any member may list them). Only fetched while
|
||||
// the window is open.
|
||||
const { data: roles } = useAiRolesQuery(windowOpen);
|
||||
@@ -254,9 +264,12 @@ export default function AiChatWindow() {
|
||||
// onResumeFallback(true); the thread disarms it on settle / local stream. The
|
||||
// window only OWNS the timer (armedAtRef stamps when it was armed for the cap).
|
||||
const [degradedPoll, setDegradedPoll] = useState(false);
|
||||
const armedAtRef = useRef(0);
|
||||
// #430: timestamp of the LAST run activity while the poll is armed — stamped on
|
||||
// arm and re-stamped whenever the polled rows change (see the effect below). The
|
||||
// idle cap is measured from this, so a long-but-progressing run keeps polling.
|
||||
const lastActivityAtRef = useRef(0);
|
||||
const onResumeFallback = useCallback((active: boolean): void => {
|
||||
if (active) armedAtRef.current = Date.now();
|
||||
if (active) lastActivityAtRef.current = Date.now();
|
||||
setDegradedPoll(active);
|
||||
}, []);
|
||||
// Reset the degraded poll whenever the open chat changes: it is scoped to the
|
||||
@@ -269,18 +282,32 @@ export default function AiChatWindow() {
|
||||
useAiChatMessagesQuery(
|
||||
activeChatId ?? undefined,
|
||||
// DELIBERATELY DUMB (invariant 8 / task 2.4): poll every 2.5s while armed
|
||||
// and under the 10-min cap; otherwise off. NO error checks (TanStack v5
|
||||
// resets fetchFailureCount each fetch, so consecutive errors are not
|
||||
// expressible — and the poll must survive a server restart) and NO tail
|
||||
// checks (the settled/local-stream semantics live in ChatThread, which
|
||||
// disarms via onResumeFallback(false)). The time cap is the only backstop.
|
||||
// and while the run is still active (#430: under the INACTIVITY cap, not a
|
||||
// fixed-from-start cap); otherwise off. NO error checks (TanStack v5 resets
|
||||
// fetchFailureCount each fetch, so consecutive errors are not expressible —
|
||||
// and the poll must survive a server restart) and NO tail checks (the
|
||||
// settled/local-stream semantics live in ChatThread, which disarms via
|
||||
// onResumeFallback(false)). The idle cap is the only backstop.
|
||||
() =>
|
||||
degradedPoll === true &&
|
||||
Date.now() - armedAtRef.current < DEGRADED_POLL_MAX_MS
|
||||
Date.now() - lastActivityAtRef.current < DEGRADED_POLL_IDLE_MAX_MS
|
||||
? 2500
|
||||
: false,
|
||||
// #344: gate on windowOpen too — no message history is fetched (and no
|
||||
// degraded poll runs) while the window is closed; it loads when the window
|
||||
// opens with an active chat.
|
||||
windowOpen,
|
||||
);
|
||||
|
||||
// #430: re-stamp the activity clock whenever the polled rows change while the
|
||||
// poll is armed. TanStack keeps the same `messageRows` reference across refetches
|
||||
// that return deep-equal data (structural sharing), so a new reference means the
|
||||
// run genuinely progressed — which extends the inactivity cap above. A stuck run
|
||||
// yields no reference change, so the cap eventually fires and stops the poll.
|
||||
useEffect(() => {
|
||||
if (degradedPoll) lastActivityAtRef.current = Date.now();
|
||||
}, [degradedPoll, messageRows]);
|
||||
|
||||
// #184 reconnect-and-live-follow. Whether detached agent runs are enabled for
|
||||
// this workspace. When the feature is off no runs are ever created, so the
|
||||
// resume attempt would only ever 204; gating ChatThread's resume on it avoids a
|
||||
@@ -315,7 +342,7 @@ export default function AiChatWindow() {
|
||||
// reads/writes via its CASL-enforced page tools using the id.
|
||||
const pageRouteMatch = useMatch("/s/:spaceSlug/p/:pageSlug");
|
||||
const pageSlug = pageRouteMatch?.params?.pageSlug;
|
||||
const { data: openPageData } = usePageQuery({
|
||||
const { data: openPageData } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
const openPage = openPageData
|
||||
|
||||
@@ -739,3 +739,170 @@ function renderResumable(initialRows: IAiChatMessageRow[]) {
|
||||
act(() => view.rerender(<Wrapper rows={rows} />));
|
||||
return { rerender, onResumeFallback };
|
||||
}
|
||||
|
||||
// #430: auto-reconnect to a DETACHED run after a LIVE SSE disconnect. The mount
|
||||
// path only resumes on mount/reload; these cover the missing trigger — a live
|
||||
// `isDisconnect` on onFinish must (backoff-)re-attach WITHOUT a reload, pin+strip
|
||||
// the live row to avoid duplicates, fall back to the degraded poll on a 204, and
|
||||
// exhaust to a manual Retry.
|
||||
describe("ChatThread — live reconnect after isDisconnect (#430)", () => {
|
||||
// A LIVE local turn that just dropped: the settled tail existed before, and the
|
||||
// partial assistant row lives only in `messages` (not persisted as a tail).
|
||||
const settledTail = () => [
|
||||
row("u1", "user", undefined, "hi"),
|
||||
row("a1", "assistant", "succeeded", "done"),
|
||||
];
|
||||
// The partial assistant message onFinish hands us for the dropped LIVE turn.
|
||||
const liveMsg = {
|
||||
id: "a2",
|
||||
role: "assistant",
|
||||
parts: [{ type: "text", text: "partial live answer" }],
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
resetState();
|
||||
// status "ready": with a live disconnect the mock is not streaming, so the
|
||||
// status==="streaming" auto-clear effect stays out of the way.
|
||||
h.state.status = "ready";
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
cleanup();
|
||||
});
|
||||
|
||||
// Render a NON-resuming mount (settled tail -> no mount resume) with autonomous
|
||||
// runs on, then simulate a live disconnect via onFinish.
|
||||
function renderLiveThenDisconnect() {
|
||||
const view = renderThread({
|
||||
autonomousRunsEnabled: true,
|
||||
initialRows: settledTail(),
|
||||
});
|
||||
// The settled tail must NOT have triggered a mount resume.
|
||||
expect(h.state.resumeStream).not.toHaveBeenCalled();
|
||||
act(() => {
|
||||
h.state.onFinish?.({
|
||||
message: liveMsg,
|
||||
isAbort: false,
|
||||
isDisconnect: true,
|
||||
isError: false,
|
||||
});
|
||||
});
|
||||
return view;
|
||||
}
|
||||
|
||||
// Fire the pending (scheduled) attempt for `attempt` (backoff = 1s,2s,4s,...).
|
||||
function advanceToAttempt(attempt: number) {
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(1000 * 2 ** (attempt - 1));
|
||||
});
|
||||
}
|
||||
|
||||
// Simulate the reconnect GET returning 204 (nothing live) so the transport's
|
||||
// no-active-stream recovery runs.
|
||||
async function reconnect204() {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn().mockResolvedValue({ status: 204, ok: false }),
|
||||
);
|
||||
await act(async () => {
|
||||
await h.state.transport!.fetch!("http://x", { method: "GET" });
|
||||
});
|
||||
}
|
||||
|
||||
// Simulate the reconnect GET returning a live 2xx stream.
|
||||
async function reconnect200() {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn().mockResolvedValue({ status: 200, ok: true }),
|
||||
);
|
||||
await act(async () => {
|
||||
await h.state.transport!.fetch!("http://x", { method: "GET" });
|
||||
});
|
||||
}
|
||||
|
||||
it("calls resumeStream POST-mount (a live disconnect triggers a backoff reconnect)", () => {
|
||||
renderLiveThenDisconnect();
|
||||
// The banner shows immediately; the attach itself fires after the first backoff.
|
||||
expect(screen.getByText(/reconnecting/i)).toBeTruthy();
|
||||
expect(h.state.resumeStream).not.toHaveBeenCalled();
|
||||
advanceToAttempt(1);
|
||||
// resumeStream is now called AFTER mount — the bug was it only ever fired once
|
||||
// on mount. The reconnect URL pins expect=live&anchor to OUR run.
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(1);
|
||||
expect(h.state.transport!.prepareReconnectToStreamRequest!().api).toBe(
|
||||
"/api/ai-chat/runs/c1/stream?expect=live&anchor=a2",
|
||||
);
|
||||
});
|
||||
|
||||
it("strips the pinned live row before replay so content is NOT duplicated", () => {
|
||||
renderLiveThenDisconnect();
|
||||
advanceToAttempt(1);
|
||||
// The attempt strips the anchor row from the store (the live replay rebuilds
|
||||
// it). Apply the setMessages updater to prove it removes exactly the anchor.
|
||||
const updater = h.state.setMessages.mock.calls.at(-1)![0] as (
|
||||
prev: { id: string }[],
|
||||
) => { id: string }[];
|
||||
expect(updater([{ id: "u1" }, { id: "a2" }])).toEqual([{ id: "u1" }]);
|
||||
});
|
||||
|
||||
it("a live re-attach (2xx) clears the reconnect banner", async () => {
|
||||
renderLiveThenDisconnect();
|
||||
advanceToAttempt(1);
|
||||
await reconnect200();
|
||||
expect(screen.queryByText(/reconnecting/i)).toBeNull();
|
||||
});
|
||||
|
||||
it("a 204 arms the degraded poll and backs off to the next attempt", async () => {
|
||||
const { onResumeFallback } = renderLiveThenDisconnect();
|
||||
advanceToAttempt(1);
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(1);
|
||||
await reconnect204();
|
||||
// Fallback engaged: the degraded poll is armed (204 -> onNoActiveStream).
|
||||
expect(onResumeFallback).toHaveBeenCalledWith(true);
|
||||
// Still reconnecting — the banner advanced to attempt 2/5.
|
||||
expect(screen.getByText(/reconnecting.*2\/5/i)).toBeTruthy();
|
||||
// The next backoff fires attempt 2 (another resumeStream).
|
||||
advanceToAttempt(2);
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("exhausts the attempt limit into a manual Retry, which restarts the sequence", async () => {
|
||||
renderLiveThenDisconnect();
|
||||
// Drive all 5 attempts, each failing with a 204.
|
||||
for (let n = 1; n <= 5; n++) {
|
||||
advanceToAttempt(n);
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(n);
|
||||
await reconnect204();
|
||||
}
|
||||
// The 5th 204 exhausted the cap -> the manual Retry replaces the banner.
|
||||
expect(screen.queryByText(/reconnecting/i)).toBeNull();
|
||||
const retry = screen.getByText("Retry");
|
||||
expect(retry).toBeTruthy();
|
||||
// Retry fires attempt 1 immediately (no backoff) — a 6th resumeStream.
|
||||
act(() => {
|
||||
fireEvent.click(retry);
|
||||
});
|
||||
expect(h.state.resumeStream).toHaveBeenCalledTimes(6);
|
||||
expect(screen.getByText(/reconnecting/i)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("does NOT reconnect when autonomous runs are disabled", () => {
|
||||
renderThread({ autonomousRunsEnabled: false, initialRows: settledTail() });
|
||||
act(() => {
|
||||
h.state.onFinish?.({
|
||||
message: liveMsg,
|
||||
isAbort: false,
|
||||
isDisconnect: true,
|
||||
isError: false,
|
||||
});
|
||||
});
|
||||
expect(screen.queryByText(/reconnecting/i)).toBeNull();
|
||||
// The terminal "connection lost" notice is shown instead (unchanged behavior).
|
||||
expect(
|
||||
screen.getByText("Connection lost — the answer was interrupted."),
|
||||
).toBeTruthy();
|
||||
advanceToAttempt(1);
|
||||
expect(h.state.resumeStream).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,7 +1,17 @@
|
||||
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
import { generateId } from "ai";
|
||||
import { ActionIcon, Box, Group, Stack, Text, Tooltip } from "@mantine/core";
|
||||
import {
|
||||
ActionIcon,
|
||||
Alert,
|
||||
Box,
|
||||
Button,
|
||||
Group,
|
||||
Loader,
|
||||
Stack,
|
||||
Text,
|
||||
Tooltip,
|
||||
} from "@mantine/core";
|
||||
import {
|
||||
IconClockHour4,
|
||||
IconPlayerPlayFilled,
|
||||
@@ -51,6 +61,15 @@ import classes from "@/features/ai-chat/components/ai-chat.module.css";
|
||||
// from the token rate.
|
||||
const STREAM_THROTTLE_MS = 50;
|
||||
|
||||
// #430: auto-reconnect after a LIVE SSE disconnect of a DETACHED (autonomous) run.
|
||||
// The run keeps executing server-side, so instead of a dead "Lost connection"
|
||||
// banner we re-attach to the live tail through the SAME resumable machinery the
|
||||
// mount path uses. Attempts back off exponentially and are capped; on exhaustion
|
||||
// the user gets a manual Retry (the degraded poll keeps catching up underneath).
|
||||
const RECONNECT_MAX_ATTEMPTS = 5;
|
||||
// Backoff before attempt N (1-based): 1s, 2s, 4s, 8s, 16s.
|
||||
const RECONNECT_BASE_DELAY_MS = 1000;
|
||||
|
||||
/** The page the user is currently viewing, sent as chat context. */
|
||||
export interface OpenPageContext {
|
||||
id: string;
|
||||
@@ -175,6 +194,10 @@ export default function ChatThread({
|
||||
const reconcileTailRef = useRef(false);
|
||||
const noStreamHandledRef = useRef(false);
|
||||
const onNoActiveStreamRef = useRef<(() => void) | null>(null);
|
||||
// #430: called from the transport's reconnect-GET success branch when a live
|
||||
// stream re-attached (2xx, not 204) — clears the reconnect banner. Kept in a ref
|
||||
// because the transport's fetch closure (useMemo([])) reads it live.
|
||||
const onReconnectAttachedRef = useRef<(() => void) | null>(null);
|
||||
// Live mount flag. The attach GET and the resumed `onFinish` are async and can
|
||||
// land AFTER this thread unmounts (the parent remounts per chat via `key`); with
|
||||
// chatIdRef then pointing at the NEW chat, an ungated late callback would arm a
|
||||
@@ -378,6 +401,10 @@ export default function ChatThread({
|
||||
// NOT drop the in-progress row or stop tracking the durable run.
|
||||
if (response.status === 204 || !response.ok)
|
||||
onNoActiveStreamRef.current?.();
|
||||
// #430: a 2xx stream re-attached (live tail or finished-replay). Signal
|
||||
// the reconnect controller to clear its banner. No-op outside an active
|
||||
// reconnect sequence (e.g. the mount attach), so it is safe here.
|
||||
else onReconnectAttachedRef.current?.();
|
||||
return response;
|
||||
} catch (err) {
|
||||
// Network throw: same no-onFinish recovery, then rethrow so the SDK
|
||||
@@ -481,6 +508,31 @@ export default function ChatThread({
|
||||
);
|
||||
}
|
||||
}
|
||||
// (2b) #430: a LIVE (non-resumed) detached run whose SSE just dropped. The
|
||||
// server run keeps executing, so instead of a dead "Lost connection" banner
|
||||
// start a reconnect sequence: pin the CURRENT streaming assistant row as the
|
||||
// strip/anchor (the live tail is the already-shown partial in `messages`, not
|
||||
// a persistent row) and re-attach to the live tail via the resumable machinery.
|
||||
const startedReconnect =
|
||||
isDisconnect &&
|
||||
!wasResumed &&
|
||||
autonomousRunsEnabled === true &&
|
||||
mountedRef.current &&
|
||||
message?.role === "assistant" &&
|
||||
typeof message.id === "string";
|
||||
if (startedReconnect) {
|
||||
beginReconnect({
|
||||
id: message.id,
|
||||
role: "assistant",
|
||||
content: "",
|
||||
status: "streaming",
|
||||
createdAt: new Date().toISOString(),
|
||||
// Preserve the partial parts so a 204 restore (onNoActiveStream) re-shows
|
||||
// what was on screen while the degraded poll catches the run up to
|
||||
// terminal (rowToUiMessage prefers metadata.parts).
|
||||
metadata: { parts: message.parts },
|
||||
});
|
||||
}
|
||||
// (3) Standard branches.
|
||||
// Forward the authoritative server chatId (streamed on the assistant
|
||||
// message metadata) so the parent adopts the REAL created chat id for a new
|
||||
@@ -490,9 +542,11 @@ export default function ChatThread({
|
||||
onTurnFinished(extractServerChatId(message), threadKey);
|
||||
// Show a neutral "stopped" marker for an aborted turn; the red error banner
|
||||
// (via `error`) already covers isError, and a clean finish clears any marker.
|
||||
// On a live disconnect that STARTED a reconnect, suppress the terminal
|
||||
// "connection lost" notice — the reconnect banner takes over (#430).
|
||||
if (isError) setStopNotice(null);
|
||||
else if (isAbort) setStopNotice("manual");
|
||||
else if (isDisconnect) setStopNotice("disconnect");
|
||||
else if (isDisconnect) setStopNotice(startedReconnect ? null : "disconnect");
|
||||
else setStopNotice(null);
|
||||
// A resumed turn NEVER flushes the queue (invariant 7): skip BOTH the
|
||||
// flush-on-abort branch and the plain flush. The local streamer is the only
|
||||
@@ -579,6 +633,106 @@ export default function ChatThread({
|
||||
|
||||
const isStreaming = status === "submitted" || status === "streaming";
|
||||
|
||||
// #430: live-disconnect reconnect controller. `null` = idle; `{ trying, attempt }`
|
||||
// = a backoff sequence is running (drives the "reconnecting… (N/max)" banner);
|
||||
// `{ failed }` = attempts exhausted (drives the manual Retry). Mirrored into a ref
|
||||
// so the transport/onNoActiveStream closures branch on the LIVE value.
|
||||
type ReconnectState =
|
||||
| null
|
||||
| { phase: "trying"; attempt: number }
|
||||
| { phase: "failed" };
|
||||
const [reconnectState, setReconnectState] = useState<ReconnectState>(null);
|
||||
const reconnectStateRef = useRef<ReconnectState>(null);
|
||||
const setReconnectStatePair = useCallback((s: ReconnectState) => {
|
||||
reconnectStateRef.current = s;
|
||||
setReconnectState(s);
|
||||
}, []);
|
||||
const reconnectTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
const clearReconnectTimer = useCallback(() => {
|
||||
if (reconnectTimerRef.current) {
|
||||
clearTimeout(reconnectTimerRef.current);
|
||||
reconnectTimerRef.current = null;
|
||||
}
|
||||
}, []);
|
||||
|
||||
// One reconnect attempt — MIRRORS the mount strip/anchor path for the LIVE case.
|
||||
// beginReconnect pinned strippedRowRef/stripRef to the run's assistant row, so:
|
||||
// - remove that row from the store (the mount path strips it from the SEED; here
|
||||
// it is already shown, so filter it out) — the live replay's `text-start` then
|
||||
// rebuilds it without DUPLICATING parts (the main dedup risk, #430);
|
||||
// - reset the one-shot 204 guard so onNoActiveStream can fire for THIS attempt;
|
||||
// - mark the turn resumed (invariant 7/8) so onFinish runs the recovery block and
|
||||
// never flushes the queue;
|
||||
// - resumeStream() -> prepareReconnectToStreamRequest builds
|
||||
// ?expect=live&anchor=<pinned id>, pinning the replay to OUR run (invariant 6).
|
||||
const attemptReconnectOnce = useCallback(
|
||||
(attempt: number) => {
|
||||
if (!mountedRef.current) return;
|
||||
const anchor = strippedRowRef.current;
|
||||
if (anchor) {
|
||||
setMessages((prev) => prev.filter((m) => m.id !== anchor.id));
|
||||
}
|
||||
noStreamHandledRef.current = false;
|
||||
setResumedTurnPair(true);
|
||||
setReconnectStatePair({ phase: "trying", attempt });
|
||||
void resumeStream();
|
||||
},
|
||||
[setMessages, setResumedTurnPair, setReconnectStatePair, resumeStream],
|
||||
);
|
||||
|
||||
// Schedule attempt `attempt` after an exponential backoff.
|
||||
const scheduleReconnectAttempt = useCallback(
|
||||
(attempt: number) => {
|
||||
clearReconnectTimer();
|
||||
setReconnectStatePair({ phase: "trying", attempt });
|
||||
reconnectTimerRef.current = setTimeout(
|
||||
() => attemptReconnectOnce(attempt),
|
||||
RECONNECT_BASE_DELAY_MS * 2 ** (attempt - 1),
|
||||
);
|
||||
},
|
||||
[clearReconnectTimer, setReconnectStatePair, attemptReconnectOnce],
|
||||
);
|
||||
|
||||
// Start a fresh reconnect sequence, pinning `anchorRow` (the live run's assistant
|
||||
// row) as the strip/anchor reused by every attempt.
|
||||
const beginReconnect = useCallback(
|
||||
(anchorRow: IAiChatMessageRow) => {
|
||||
if (!autonomousRunsEnabled || !mountedRef.current) return;
|
||||
strippedRowRef.current = anchorRow;
|
||||
stripRef.current = true;
|
||||
scheduleReconnectAttempt(1);
|
||||
},
|
||||
[autonomousRunsEnabled, scheduleReconnectAttempt],
|
||||
);
|
||||
|
||||
// Manual Retry (shown once attempts are exhausted): restart at attempt 1 and fire
|
||||
// immediately (the user asked for it now — no backoff).
|
||||
const retryReconnect = useCallback(() => {
|
||||
clearReconnectTimer();
|
||||
attemptReconnectOnce(1);
|
||||
}, [clearReconnectTimer, attemptReconnectOnce]);
|
||||
|
||||
// Live SSE re-attached (the reconnect GET returned a 2xx stream): clear the
|
||||
// banner + any pending backoff. No-op outside a sequence (e.g. the mount attach).
|
||||
const onReconnectAttached = useCallback(() => {
|
||||
if (!mountedRef.current || !reconnectStateRef.current) return;
|
||||
clearReconnectTimer();
|
||||
setReconnectStatePair(null);
|
||||
}, [clearReconnectTimer, setReconnectStatePair]);
|
||||
onReconnectAttachedRef.current = onReconnectAttached;
|
||||
|
||||
// The reconnect GET could not attach (204 / error). onNoActiveStream has already
|
||||
// armed the degraded poll (the robust fallback that drives the row to terminal
|
||||
// from the DB), so this only decides the LIVE-attach retry: back off and try
|
||||
// again up to the cap, else surface the manual Retry.
|
||||
const onReconnectNoStream = useCallback(() => {
|
||||
const s = reconnectStateRef.current;
|
||||
if (s?.phase !== "trying") return;
|
||||
if (s.attempt < RECONNECT_MAX_ATTEMPTS)
|
||||
scheduleReconnectAttempt(s.attempt + 1);
|
||||
else setReconnectStatePair({ phase: "failed" });
|
||||
}, [scheduleReconnectAttempt, setReconnectStatePair]);
|
||||
|
||||
// 204-handler (`onNoActiveStream`): the attach returned 204 — nothing live to
|
||||
// resume (overflow / begin-failure / after retention / anchor-mismatch). One-
|
||||
// shot via noStreamHandledRef (we do NOT null onNoActiveStreamRef). Exactly four
|
||||
@@ -610,7 +764,17 @@ export default function ChatThread({
|
||||
// (d) 204 means onFinish will NOT fire — clear the suppression flag so it
|
||||
// cannot swallow the NEXT local turn's queue flush.
|
||||
setResumedTurnPair(false);
|
||||
}, [setMessages, queryClient, onResumeFallback, setResumedTurnPair]);
|
||||
// (e) #430: if this 204/error landed during a live-disconnect reconnect
|
||||
// sequence, back off and retry the live attach (or give up to the manual
|
||||
// Retry). The degraded poll armed in (c) is the fallback either way.
|
||||
onReconnectNoStream();
|
||||
}, [
|
||||
setMessages,
|
||||
queryClient,
|
||||
onResumeFallback,
|
||||
setResumedTurnPair,
|
||||
onReconnectNoStream,
|
||||
]);
|
||||
onNoActiveStreamRef.current = onNoActiveStream;
|
||||
|
||||
// Mount effect: kick off the resume attempt for a non-settled tail. Marking the
|
||||
@@ -628,6 +792,9 @@ export default function ChatThread({
|
||||
return () => {
|
||||
mountedRef.current = false;
|
||||
attachAbortRef.current?.abort();
|
||||
// #430: drop any pending reconnect backoff so it can't fire against the next
|
||||
// chat this thread's refs are reused for.
|
||||
if (reconnectTimerRef.current) clearTimeout(reconnectTimerRef.current);
|
||||
};
|
||||
// Mount-only by design; the parent remounts per chat via `key`.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
@@ -666,12 +833,27 @@ export default function ChatThread({
|
||||
if (tail.status !== "streaming") {
|
||||
reconcileTailRef.current = false;
|
||||
onResumeFallback?.(false);
|
||||
// #430: the run reached its terminal state via the degraded poll — there is
|
||||
// no live tail left to reconnect to, so drop any reconnect banner / Retry.
|
||||
clearReconnectTimer();
|
||||
setReconnectStatePair(null);
|
||||
}
|
||||
// onResumeFallback intentionally omitted (parent-stable callback); deps are
|
||||
// fixed by the resume design.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [initialRows, isStreaming, setMessages]);
|
||||
|
||||
// #430: a real stream is live again — the reconnect re-attached to the live tail
|
||||
// (status -> "streaming") OR the user started a new local turn. Either way clear
|
||||
// the reconnect banner + any pending backoff. Gated on "streaming" (not the
|
||||
// broader "submitted") so a still-pending attach GET does not clear prematurely.
|
||||
useEffect(() => {
|
||||
if (status === "streaming") {
|
||||
clearReconnectTimer();
|
||||
setReconnectStatePair(null);
|
||||
}
|
||||
}, [status, clearReconnectTimer, setReconnectStatePair]);
|
||||
|
||||
// "Send now" on a queued message: interrupt the current turn and immediately
|
||||
// send THIS message, keeping the agent's partial output. Other queued messages
|
||||
// stay queued and flush normally after the new turn. Reuses the existing
|
||||
@@ -719,6 +901,9 @@ export default function ChatThread({
|
||||
// observer's Stop would otherwise leave the attach fetch running.
|
||||
attachAbortRef.current?.abort();
|
||||
stop();
|
||||
// #430: pressing Stop also cancels an in-progress reconnect sequence.
|
||||
clearReconnectTimer();
|
||||
setReconnectStatePair(null);
|
||||
if (!autonomousRunsEnabled) return;
|
||||
if (chatIdRef.current) {
|
||||
onServerStop?.(chatIdRef.current);
|
||||
@@ -740,7 +925,13 @@ export default function ChatThread({
|
||||
// for this fix. Documented so a future change can address the abort-ordering.
|
||||
stopPendingRef.current = true;
|
||||
}
|
||||
}, [stop, autonomousRunsEnabled, onServerStop]);
|
||||
}, [
|
||||
stop,
|
||||
autonomousRunsEnabled,
|
||||
onServerStop,
|
||||
clearReconnectTimer,
|
||||
setReconnectStatePair,
|
||||
]);
|
||||
|
||||
// Clear the stopped marker as soon as a new turn begins streaming, and drop any
|
||||
// stale "Send now" interrupt flags. On the legit interrupt path both refs are
|
||||
@@ -825,6 +1016,43 @@ export default function ChatThread({
|
||||
detail={errorView.detail}
|
||||
mb="xs"
|
||||
/>
|
||||
) : reconnectState ? (
|
||||
// #430: while auto-reconnecting to a detached run's live tail, show progress
|
||||
// instead of a dead "Lost connection" banner; once attempts are exhausted,
|
||||
// offer a manual Retry (the degraded poll keeps catching up underneath).
|
||||
<Alert
|
||||
variant="light"
|
||||
color="gray"
|
||||
p="xs"
|
||||
mb="xs"
|
||||
style={{ flexShrink: 0 }}
|
||||
>
|
||||
<Group gap={8} wrap="nowrap" align="center">
|
||||
{reconnectState.phase === "trying" ? (
|
||||
<>
|
||||
<Loader size={14} color="gray" style={{ flex: "none" }} />
|
||||
<Text size="sm" lh={1.3} c="dimmed">
|
||||
{t("Connection lost — reconnecting…")}
|
||||
{` (${reconnectState.attempt}/${RECONNECT_MAX_ATTEMPTS})`}
|
||||
</Text>
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<Text size="sm" lh={1.3} c="dimmed" style={{ flex: 1 }}>
|
||||
{t("Couldn't reconnect to the answer.")}
|
||||
</Text>
|
||||
<Button
|
||||
size="compact-xs"
|
||||
variant="light"
|
||||
color="gray"
|
||||
onClick={retryReconnect}
|
||||
>
|
||||
{t("Retry")}
|
||||
</Button>
|
||||
</>
|
||||
)}
|
||||
</Group>
|
||||
</Alert>
|
||||
) : stopNotice ? (
|
||||
<ChatStoppedNotice
|
||||
text={
|
||||
|
||||
@@ -53,8 +53,12 @@ export const AI_CHAT_MESSAGES_RQ_KEY = (chatId: string) => [
|
||||
chatId,
|
||||
];
|
||||
|
||||
/** Paginated list of the current user's chats (auto-loads further pages). */
|
||||
export function useAiChatsQuery() {
|
||||
/**
|
||||
* Paginated list of the current user's chats (auto-loads further pages).
|
||||
* `enabled` (default true) lets the AI chat window skip fetching while it is
|
||||
* closed — the list is only needed once the window is open.
|
||||
*/
|
||||
export function useAiChatsQuery(enabled: boolean = true) {
|
||||
const query = useInfiniteQuery({
|
||||
queryKey: AI_CHATS_RQ_KEY,
|
||||
queryFn: ({ pageParam }) => getAiChats({ cursor: pageParam, limit: 50 }),
|
||||
@@ -63,6 +67,7 @@ export function useAiChatsQuery() {
|
||||
lastPage.meta.hasNextPage
|
||||
? (lastPage.meta.nextCursor ?? undefined)
|
||||
: undefined,
|
||||
enabled,
|
||||
});
|
||||
|
||||
const data = useMemo<IPagination<IAiChat> | undefined>(() => {
|
||||
@@ -93,6 +98,9 @@ export function useAiChatMessagesQuery(
|
||||
// follow the detached run to settle. The callback form lives in AiChatWindow;
|
||||
// threaded here verbatim so this query owns the polling. Undefined => no poll.
|
||||
refetchInterval?: number | false | (() => number | false),
|
||||
// #344: gate the query so a backgrounded/hidden window stops issuing refetches
|
||||
// and duplicating work. Defaults to enabled to preserve existing call-sites.
|
||||
enabled: boolean = true,
|
||||
) {
|
||||
const query = useInfiniteQuery({
|
||||
queryKey: AI_CHAT_MESSAGES_RQ_KEY(chatId ?? ""),
|
||||
@@ -103,7 +111,7 @@ export function useAiChatMessagesQuery(
|
||||
lastPage.meta.hasNextPage
|
||||
? (lastPage.meta.nextCursor ?? undefined)
|
||||
: undefined,
|
||||
enabled: !!chatId,
|
||||
enabled: !!chatId && enabled,
|
||||
refetchInterval,
|
||||
});
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ vi.mock("@/features/comment/components/comment-editor", () => ({
|
||||
// case renders in isolation.
|
||||
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
usePageQuery: () => ({ data: undefined, isLoading: false, isError: false }),
|
||||
usePageMetaQuery: () => ({ data: undefined, isLoading: false, isError: false }),
|
||||
}));
|
||||
vi.mock("@/features/share/queries/share-query.ts", () => ({
|
||||
useSharePageQuery: () => ({ data: undefined }),
|
||||
|
||||
@@ -22,7 +22,7 @@ import CommentEditor from "@/features/comment/components/comment-editor";
|
||||
import CommentActions from "@/features/comment/components/comment-actions";
|
||||
import { useFocusWithin } from "@mantine/hooks";
|
||||
import { IComment } from "@/features/comment/types/comment.types.ts";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useGetSpaceBySlugQuery } from "@/features/space/queries/space-query.ts";
|
||||
@@ -56,7 +56,7 @@ export function buildChildrenByParent(
|
||||
function CommentListWithTabs({ onClose }: CommentListWithTabsProps) {
|
||||
const { t } = useTranslation();
|
||||
const { pageSlug } = useParams();
|
||||
const { data: page } = usePageQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
const { data: page } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
const {
|
||||
data: comments,
|
||||
isLoading: isCommentsLoading,
|
||||
|
||||
@@ -49,19 +49,14 @@ export default function FootnoteDefinitionView(props: NodeViewProps) {
|
||||
className={classes.definition}
|
||||
style={{ ["--footnote-number" as any]: `"${number}"` }}
|
||||
>
|
||||
{/* #146: contentDOM MUST be the first child — a non-editable marker before
|
||||
{/* #146: contentDOM MUST be the first child — non-editable chrome before
|
||||
it makes click hit-testing snap the caret above. Content first; the
|
||||
marker + back-link follow in DOM and are placed left/right via CSS
|
||||
flex `order`. The second #146 mitigation lives in
|
||||
back-link follows in DOM and is placed on the right via CSS flex. The
|
||||
decorative "N." number is rendered inline via the .definitionContent
|
||||
::before rule (from the --footnote-number var), so no marker element
|
||||
precedes the content. The second #146 mitigation lives in
|
||||
editor-paste-handler.tsx (reflowAfterPaste). */}
|
||||
<NodeViewContent className={classes.definitionContent} />
|
||||
<span
|
||||
className={classes.definitionMarker}
|
||||
contentEditable={false}
|
||||
aria-hidden="true"
|
||||
>
|
||||
{number}.
|
||||
</span>
|
||||
{refCount > 1 ? (
|
||||
// Multiple references -> ↩ followed by one lettered link per occurrence.
|
||||
<span
|
||||
|
||||
@@ -81,34 +81,34 @@
|
||||
.definition {
|
||||
display: flex;
|
||||
align-items: flex-start;
|
||||
/* Tight number→text spacing (~one space) so it reads like "1. text"
|
||||
instead of leaving a wide gap after the period. */
|
||||
gap: 0.4em;
|
||||
/* Tight spacing between the content and the trailing ↩ back-link. */
|
||||
gap: 0.3em;
|
||||
padding: 2px 0;
|
||||
/* Footnotes read smaller than body text (16px). Matches .listHeading. */
|
||||
font-size: var(--mantine-font-size-sm);
|
||||
}
|
||||
|
||||
.definitionMarker {
|
||||
order: -1; /* keep the "N." marker on the LEFT though it follows content in DOM (#146) */
|
||||
flex: 0 0 auto;
|
||||
min-width: 1.5em;
|
||||
/* Right-align within the narrow column so the period sits next to the text
|
||||
and multi-digit numbers (10, 11, …) stay aligned on their right edge. */
|
||||
text-align: right;
|
||||
font-variant-numeric: tabular-nums;
|
||||
color: var(--mantine-color-dimmed);
|
||||
user-select: none;
|
||||
}
|
||||
|
||||
/* The "N." number is decorative (from the --footnote-number CSS var on the
|
||||
wrapper, never in the document model) and is rendered inline at the start of
|
||||
the first content line via ::before. This keeps text and wrapped lines flush
|
||||
to the left margin — no hanging indent — while the editable contentDOM stays
|
||||
the FIRST DOM child (#146). */
|
||||
.definitionContent {
|
||||
flex: 1 1 auto;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
/* The inner editable paragraph inherits `.ProseMirror p { margin: 0.5em 0 }`,
|
||||
which pushes the first text line ~0.5em below the "N." marker (aligned to
|
||||
flex-start), making the number float above the text. Drop the outer margins
|
||||
so the marker and the first line share the same top edge — same approach
|
||||
used for callouts in core.css. */
|
||||
.definitionContent > :first-child::before {
|
||||
content: var(--footnote-number, "?") ". ";
|
||||
color: var(--mantine-color-dimmed);
|
||||
font-variant-numeric: tabular-nums;
|
||||
user-select: none;
|
||||
}
|
||||
|
||||
/* The inner editable paragraph inherits `.ProseMirror p { margin: 0.5em 0 }`.
|
||||
Drop the outer margins so the definition sits tight to the heading above and
|
||||
the ::before number aligns with the top of the row — same approach used for
|
||||
callouts in core.css. */
|
||||
.definitionContent > :first-child {
|
||||
margin-top: 0;
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ import classes from "./link.module.css";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { INTERNAL_LINK_REGEX } from "@/lib/constants";
|
||||
import { LinkEditorPanel } from "@/features/editor/components/link/link-editor-panel.tsx";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { useSharePageQuery } from "@/features/share/queries/share-query.ts";
|
||||
import { buildSharedPageUrl } from "@/features/page/page.utils.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
@@ -83,7 +83,7 @@ export default function LinkView(props: MarkViewProps) {
|
||||
const isPopoverVisible = popoverState !== "closed";
|
||||
const activeView = isPopoverVisible ? popoverState : lastOpenState.current;
|
||||
|
||||
const { data: linkedPage } = usePageQuery({
|
||||
const { data: linkedPage } = usePageMetaQuery({
|
||||
pageId: isPopoverVisible && slugId && !isShareRoute ? slugId : null,
|
||||
});
|
||||
|
||||
|
||||
@@ -25,7 +25,7 @@ import { IconFileDescription, IconPlus } from "@tabler/icons-react";
|
||||
import { useSpaceQuery } from "@/features/space/queries/space-query.ts";
|
||||
import { useParams } from "react-router-dom";
|
||||
import { v7 as uuid7 } from "uuid";
|
||||
import { useAtom } from "jotai";
|
||||
import { useAtom, useSetAtom, useStore } from "jotai";
|
||||
import { currentUserAtom } from "@/features/user/atoms/current-user-atom.ts";
|
||||
import {
|
||||
MentionListProps,
|
||||
@@ -34,7 +34,7 @@ import {
|
||||
import { IPage } from "@/features/page/types/page.types";
|
||||
import {
|
||||
useCreatePageMutation,
|
||||
usePageQuery,
|
||||
usePageMetaQuery,
|
||||
} from "@/features/page/queries/page-query";
|
||||
import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom";
|
||||
import { treeModel } from "@/features/page/tree/model/tree-model";
|
||||
@@ -50,12 +50,16 @@ const MentionList = forwardRef<any, MentionListProps>((props, ref) => {
|
||||
const [countAnnouncement, setCountAnnouncement] = useState("");
|
||||
const [selectionAnnouncement, setSelectionAnnouncement] = useState("");
|
||||
const { pageSlug, spaceSlug } = useParams();
|
||||
const { data: page } = usePageQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
const { data: page } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
const { data: space } = useSpaceQuery(spaceSlug);
|
||||
const [currentUser] = useAtom(currentUserAtom);
|
||||
const [renderItems, setRenderItems] = useState<MentionSuggestionItem[]>([]);
|
||||
const { t } = useTranslation();
|
||||
const [data, setData] = useAtom(treeDataAtom);
|
||||
// Setter-only: the tree value is read only imperatively inside createPage
|
||||
// (via `store` below), never at render, so useSetAtom avoids re-rendering the
|
||||
// mention popup on any tree event.
|
||||
const setData = useSetAtom(treeDataAtom);
|
||||
const store = useStore();
|
||||
const createPageMutation = useCreatePageMutation();
|
||||
const emit = useQueryEmit();
|
||||
const isInCommentContext = props.isInCommentContext ?? false;
|
||||
@@ -272,9 +276,11 @@ const MentionList = forwardRef<any, MentionListProps>((props, ref) => {
|
||||
children: [],
|
||||
};
|
||||
|
||||
const lastIndex = data.length;
|
||||
// Read the live tree imperatively at call time.
|
||||
const currentTree = store.get(treeDataAtom);
|
||||
const lastIndex = currentTree.length;
|
||||
|
||||
setData(treeModel.insert(data, parentId, newNode, lastIndex));
|
||||
setData(treeModel.insert(currentTree, parentId, newNode, lastIndex));
|
||||
|
||||
props.command({
|
||||
id: uuid7(),
|
||||
|
||||
@@ -2,7 +2,7 @@ import { NodeViewProps, NodeViewWrapper } from "@tiptap/react";
|
||||
import { ActionIcon, Anchor, Text } from "@mantine/core";
|
||||
import { IconFileDescription } from "@tabler/icons-react";
|
||||
import { Link, useLocation, useNavigate, useParams } from "react-router-dom";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { useSharePageQuery } from "@/features/share/queries/share-query.ts";
|
||||
import {
|
||||
buildPageUrl,
|
||||
@@ -36,7 +36,7 @@ export function MentionContent({ attrs }: { attrs: MentionAttrs }) {
|
||||
data: page,
|
||||
isLoading,
|
||||
isError,
|
||||
} = usePageQuery({ pageId: isPageMention && !isShareRoute ? slugId : null });
|
||||
} = usePageMetaQuery({ pageId: isPageMention && !isShareRoute ? slugId : null });
|
||||
|
||||
const { data: sharedPage } = useSharePageQuery({
|
||||
pageId: isPageMention && isShareRoute ? slugId : undefined,
|
||||
|
||||
@@ -24,7 +24,6 @@ export function useFavoritesQuery(type?: FavoriteType, spaceId?: string) {
|
||||
initialPageParam: undefined as string | undefined,
|
||||
getNextPageParam: (lastPage) =>
|
||||
lastPage.meta.hasNextPage ? lastPage.meta.nextCursor : undefined,
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -32,7 +31,6 @@ export function useFavoriteIds(type: FavoriteType, spaceId?: string): Set<string
|
||||
const { data } = useQuery({
|
||||
queryKey: ["favorite-ids", type, spaceId],
|
||||
queryFn: () => getFavoriteIds(type, spaceId),
|
||||
refetchOnMount: true,
|
||||
});
|
||||
|
||||
const items = data?.items;
|
||||
|
||||
@@ -12,7 +12,7 @@ import { useAtomValue } from "jotai";
|
||||
import { useParams } from "react-router-dom";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { pageEditorAtom } from "@/features/editor/atoms/editor-atoms.ts";
|
||||
import { useBacklinksCountQuery } from "@/features/page-details/queries/backlinks-query.ts";
|
||||
import { BacklinksModal } from "./backlinks-modal";
|
||||
@@ -23,7 +23,7 @@ import { LabelsSection } from "@/features/label/components/labels-section.tsx";
|
||||
|
||||
export function PageDetailsAside() {
|
||||
const { pageSlug } = useParams();
|
||||
const { data: page } = usePageQuery({
|
||||
const { data: page } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
const pageEditor = useAtomValue(pageEditorAtom);
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { SpaceTreeNode } from "@/features/page/tree/types";
|
||||
|
||||
// breadcrumb.tsx transitively imports @/main.tsx (via usePageMetaQuery ->
|
||||
// queryClient), whose module body calls ReactDOM.createRoot on a null root in
|
||||
// jsdom. Stub it so importing the pure helper under test doesn't run that
|
||||
// (breadcrumbPathEqual does not use queryClient, so a dummy is enough).
|
||||
vi.mock("@/main.tsx", () => ({ queryClient: {} }));
|
||||
|
||||
import { breadcrumbPathEqual } from "./breadcrumb";
|
||||
|
||||
// breadcrumbPathEqual is the ONLY point where a false-positive equality would
|
||||
// leave a stale/incorrect breadcrumb trail on screen: it decides whether the
|
||||
// selectAtom hands back the same reference (no re-render) for the ancestor chain.
|
||||
// Pin both directions — a too-loose equality goes stale on a rename; a too-tight
|
||||
// one loses the perf win.
|
||||
const node = (over: Partial<SpaceTreeNode>): SpaceTreeNode =>
|
||||
({ id: "a", slugId: "sa", name: "A", icon: "📄", ...over }) as SpaceTreeNode;
|
||||
|
||||
describe("breadcrumbPathEqual", () => {
|
||||
it("both null → true", () => {
|
||||
expect(breadcrumbPathEqual(null, null)).toBe(true);
|
||||
});
|
||||
|
||||
it("same reference → true", () => {
|
||||
const p = [node({})];
|
||||
expect(breadcrumbPathEqual(p, p)).toBe(true);
|
||||
});
|
||||
|
||||
it("equal by id/slugId/name/icon (different arrays) → true", () => {
|
||||
expect(breadcrumbPathEqual([node({})], [node({})])).toBe(true);
|
||||
});
|
||||
|
||||
it("one side null → false", () => {
|
||||
expect(breadcrumbPathEqual([node({})], null)).toBe(false);
|
||||
expect(breadcrumbPathEqual(null, [node({})])).toBe(false);
|
||||
});
|
||||
|
||||
it("different length → false", () => {
|
||||
expect(
|
||||
breadcrumbPathEqual([node({})], [node({}), node({ id: "b" })]),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it.each(["name", "icon", "slugId", "id"] as const)(
|
||||
"a changed %s → false (breadcrumb must re-render)",
|
||||
(field) => {
|
||||
expect(
|
||||
breadcrumbPathEqual([node({})], [node({ [field]: "CHANGED" })]),
|
||||
).toBe(false);
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -1,7 +1,9 @@
|
||||
import { useAtomValue } from "jotai";
|
||||
import { selectAtom } from "jotai/utils";
|
||||
import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
|
||||
import React, { useCallback, useEffect, useState } from "react";
|
||||
import React, { useCallback, useEffect, useMemo, useState } from "react";
|
||||
import { computeBreadcrumbState } from "./breadcrumb.utils";
|
||||
import { findBreadcrumbPath } from "@/features/page/tree/utils";
|
||||
import {
|
||||
Button,
|
||||
Anchor,
|
||||
@@ -18,7 +20,7 @@ import { SpaceTreeNode } from "@/features/page/tree/types.ts";
|
||||
import { IPage } from "@/features/page/types/page.types.ts";
|
||||
import { buildPageUrl } from "@/features/page/page.utils.ts";
|
||||
import {
|
||||
usePageQuery,
|
||||
usePageMetaQuery,
|
||||
usePageBreadcrumbsQuery,
|
||||
} from "@/features/page/queries/page-query.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
@@ -32,39 +34,84 @@ function getTitle(name: string, icon: string) {
|
||||
return name;
|
||||
}
|
||||
|
||||
/**
|
||||
* Equality over a breadcrumb chain by the only fields the breadcrumb renders
|
||||
* (id, slugId, name, icon). Lets the selectAtom below hand back the SAME
|
||||
* reference when an unrelated tree mutation leaves THIS page's ancestor chain
|
||||
* visually unchanged, so the breadcrumb no longer re-renders on every tree
|
||||
* event (it previously subscribed to the whole treeDataAtom).
|
||||
*/
|
||||
export function breadcrumbPathEqual(
|
||||
a: SpaceTreeNode[] | null,
|
||||
b: SpaceTreeNode[] | null,
|
||||
): boolean {
|
||||
if (a === b) return true;
|
||||
if (!a || !b || a.length !== b.length) return false;
|
||||
for (let i = 0; i < a.length; i++) {
|
||||
if (
|
||||
a[i].id !== b[i].id ||
|
||||
a[i].slugId !== b[i].slugId ||
|
||||
a[i].name !== b[i].name ||
|
||||
a[i].icon !== b[i].icon
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export default function Breadcrumb() {
|
||||
const { t } = useTranslation();
|
||||
const treeData = useAtomValue(treeDataAtom);
|
||||
const [breadcrumbNodes, setBreadcrumbNodes] = useState<
|
||||
SpaceTreeNode[] | null
|
||||
>(null);
|
||||
const { pageSlug, spaceSlug } = useParams();
|
||||
const { data: currentPage } = usePageQuery({
|
||||
const { data: currentPage } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
const currentPageId = currentPage?.id;
|
||||
// The page's own ancestor chain, fetched independently of the lazily-built
|
||||
// sidebar tree so a deep page doesn't render a blank breadcrumb for seconds
|
||||
// while the tree backfills (#218).
|
||||
const { data: ancestors } = usePageBreadcrumbsQuery(currentPage?.id);
|
||||
const { data: ancestors } = usePageBreadcrumbsQuery(currentPageId);
|
||||
const isMobile = useMediaQuery("(max-width: 48em)");
|
||||
|
||||
// Narrowed subscription: instead of subscribing to the whole treeDataAtom and
|
||||
// recomputing on every tree event, derive ONLY the current page's ancestor
|
||||
// chain. The custom equality returns the previous reference when that chain is
|
||||
// visually unchanged, so an unrelated tree mutation no longer re-renders this
|
||||
// component. Mirrors computeBreadcrumbState's tree-hit branch
|
||||
// (findBreadcrumbPath); the tree-miss/ancestors fallback is applied below.
|
||||
const treePathAtom = useMemo(
|
||||
() =>
|
||||
selectAtom(
|
||||
treeDataAtom,
|
||||
(tree): SpaceTreeNode[] | null =>
|
||||
currentPageId ? findBreadcrumbPath(tree, currentPageId) : null,
|
||||
breadcrumbPathEqual,
|
||||
),
|
||||
[currentPageId],
|
||||
);
|
||||
const treePath = useAtomValue(treePathAtom);
|
||||
|
||||
useEffect(() => {
|
||||
if (!currentPage) return;
|
||||
|
||||
// Selection/mapping + stale-clearing live in a pure, unit-tested helper
|
||||
// (#218). It resolves the correct chain when possible and, on a transient
|
||||
// miss, clears a chain left over from a previously-viewed page instead of
|
||||
// showing the wrong trail — while keeping a chain already resolved for THIS
|
||||
// page to avoid a blank flash.
|
||||
// (#218). The tree-hit chain (treePath) always wins when present; otherwise
|
||||
// fall back to the page's own ancestors and the stale-clearing logic — this
|
||||
// reproduces computeBreadcrumbState(fullTree, ancestors, …) exactly, since
|
||||
// its tree-hit branch is precisely findBreadcrumbPath(fullTree, pageId).
|
||||
setBreadcrumbNodes((previous) =>
|
||||
treePath ??
|
||||
computeBreadcrumbState(
|
||||
treeData,
|
||||
null,
|
||||
ancestors as IPage[] | undefined,
|
||||
currentPage.id,
|
||||
previous,
|
||||
),
|
||||
);
|
||||
}, [currentPage?.id, treeData, ancestors]);
|
||||
}, [currentPage?.id, treePath, ancestors]);
|
||||
|
||||
const HiddenNodesTooltipContent = () =>
|
||||
breadcrumbNodes?.slice(1, -1).map((node) => (
|
||||
|
||||
@@ -24,7 +24,7 @@ import { historyAtoms } from "@/features/page-history/atoms/history-atoms.ts";
|
||||
import { useDisclosure, useHotkeys } from "@mantine/hooks";
|
||||
import { useClipboard } from "@/hooks/use-clipboard";
|
||||
import { useParams } from "react-router-dom";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import {
|
||||
useToggleTemporaryMutation,
|
||||
syncTemporaryExpiresInCache,
|
||||
@@ -67,7 +67,7 @@ export default function PageHeaderMenu({ readOnly }: PageHeaderMenuProps) {
|
||||
const commentsTriggerProps = useAsideTriggerProps("comments");
|
||||
const tocTriggerProps = useAsideTriggerProps("toc");
|
||||
const { pageSlug } = useParams();
|
||||
const { data: page } = usePageQuery({
|
||||
const { data: page } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
const isDeleted = !!page?.deletedAt;
|
||||
@@ -146,7 +146,7 @@ function PageActionMenu({ readOnly }: PageActionMenuProps) {
|
||||
const [, setHistoryModalOpen] = useAtom(historyAtoms);
|
||||
const clipboard = useClipboard({ timeout: 500 });
|
||||
const { pageSlug, spaceSlug } = useParams();
|
||||
const { data: page, isLoading } = usePageQuery({
|
||||
const { data: page, isLoading } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
const { handleDelete } = useTreeMutation(page?.spaceId ?? "");
|
||||
|
||||
@@ -10,7 +10,7 @@ import { IconClockHour4, IconTrash } from "@tabler/icons-react";
|
||||
import { useState } from "react";
|
||||
import { Trans, useTranslation } from "react-i18next";
|
||||
import { useTimeAgo } from "@/hooks/use-time-ago.tsx";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { useTreeMutation } from "@/features/page/tree/hooks/use-tree-mutation.ts";
|
||||
import {
|
||||
useToggleTemporaryMutation,
|
||||
@@ -35,7 +35,7 @@ type TemporaryNoteBannerProps = {
|
||||
*/
|
||||
export function TemporaryNoteBanner({ slugId }: TemporaryNoteBannerProps) {
|
||||
const { t } = useTranslation();
|
||||
const { data: page } = usePageQuery({ pageId: slugId });
|
||||
const { data: page } = usePageMetaQuery({ pageId: slugId });
|
||||
const { data: space } = useGetSpaceBySlugQuery(page?.space?.slug);
|
||||
const spaceAbility = useSpaceAbility(space?.membership?.permissions);
|
||||
const expiresTimeAgo = useTimeAgo(page?.temporaryExpiresAt);
|
||||
|
||||
@@ -0,0 +1,149 @@
|
||||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import type { IPage } from "@/features/page/types/page.types";
|
||||
|
||||
// A fresh QueryClient stands in for the app singleton (importing the real
|
||||
// @/main.tsx would run ReactDOM.createRoot, which has no DOM root in jsdom). The
|
||||
// factory constructs it (QueryClient can't be referenced in vi.hoisted — that
|
||||
// runs before imports resolve); we import the SAME mocked instance back to seed
|
||||
// and assert on it.
|
||||
vi.mock("@/main.tsx", async () => {
|
||||
const { QueryClient } = await import("@tanstack/react-query");
|
||||
return { queryClient: new QueryClient() };
|
||||
});
|
||||
|
||||
import { queryClient as h_qc } from "@/main.tsx";
|
||||
import { invalidateOnUpdatePage } from "./page-query";
|
||||
|
||||
const h = { qc: h_qc };
|
||||
|
||||
// invalidateOnUpdatePage is the field-only (title/icon) tree path: instead of a
|
||||
// blanket invalidate it patches the affected node IN PLACE in every cached embed
|
||||
// subtree. The undefined-guard is LOAD-BEARING: a title-only socket event carries
|
||||
// icon:undefined, and without the guard `{...p, icon: undefined}` would WIPE the
|
||||
// icon in every cached subtree.
|
||||
const page = (over: Partial<IPage>): IPage =>
|
||||
({ id: "p1", title: "Old", icon: "📄", spaceId: "s1" }) as IPage &
|
||||
typeof over as IPage;
|
||||
|
||||
describe("invalidateOnUpdatePage — pointwise embed-cache patch", () => {
|
||||
beforeEach(() => {
|
||||
h.qc.clear();
|
||||
});
|
||||
|
||||
it("title-only event updates title but PRESERVES the icon (undefined-guard)", () => {
|
||||
const key = ["page-tree", "parent-1"];
|
||||
h.qc.setQueryData<IPage[]>(key, [
|
||||
{ id: "p1", title: "Old", icon: "📄", spaceId: "s1" } as IPage,
|
||||
{ id: "p2", title: "Other", icon: "📁", spaceId: "s1" } as IPage,
|
||||
]);
|
||||
|
||||
// icon passed as undefined (a title-only update)
|
||||
invalidateOnUpdatePage(
|
||||
"s1",
|
||||
"parent-1",
|
||||
"p1",
|
||||
"New Title",
|
||||
undefined as unknown as string,
|
||||
);
|
||||
|
||||
const patched = h.qc.getQueryData<IPage[]>(key)!;
|
||||
const p1 = patched.find((p) => p.id === "p1")!;
|
||||
const p2 = patched.find((p) => p.id === "p2")!;
|
||||
expect(p1.title).toBe("New Title");
|
||||
expect(p1.icon).toBe("📄"); // preserved, not wiped
|
||||
// Sibling node untouched.
|
||||
expect(p2.title).toBe("Other");
|
||||
expect(p2.icon).toBe("📁");
|
||||
});
|
||||
|
||||
it("icon-only event updates icon but preserves the title", () => {
|
||||
const key = ["page-tree", "parent-1"];
|
||||
h.qc.setQueryData<IPage[]>(key, [
|
||||
{ id: "p1", title: "Keep", icon: "📄", spaceId: "s1" } as IPage,
|
||||
]);
|
||||
|
||||
invalidateOnUpdatePage(
|
||||
"s1",
|
||||
"parent-1",
|
||||
"p1",
|
||||
undefined as unknown as string,
|
||||
"🚀",
|
||||
);
|
||||
|
||||
const p1 = h.qc.getQueryData<IPage[]>(key)!.find((p) => p.id === "p1")!;
|
||||
expect(p1.icon).toBe("🚀");
|
||||
expect(p1.title).toBe("Keep");
|
||||
});
|
||||
|
||||
// The sidebar-pages cache (InfiniteData) is patched on the same event. It must
|
||||
// carry the SAME undefined-guard as the embed path above — otherwise a
|
||||
// title-only event's icon:undefined would wipe the sidebar entry's icon.
|
||||
const sidebarKey = ["sidebar-pages", { pageId: "parent-1", spaceId: "s1" }];
|
||||
const seedSidebar = () =>
|
||||
h.qc.setQueryData(sidebarKey, {
|
||||
pageParams: [undefined],
|
||||
pages: [
|
||||
{
|
||||
items: [
|
||||
{ id: "p1", title: "Old", icon: "📄", spaceId: "s1" } as IPage,
|
||||
{ id: "p2", title: "Other", icon: "📁", spaceId: "s1" } as IPage,
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
const sidebarItem = (id: string) => {
|
||||
const data = h.qc.getQueryData(sidebarKey) as {
|
||||
pages: { items: IPage[] }[];
|
||||
};
|
||||
return data.pages[0].items.find((p) => p.id === id)!;
|
||||
};
|
||||
|
||||
it("sidebar cache: title-only event updates title but PRESERVES the icon", () => {
|
||||
seedSidebar();
|
||||
|
||||
invalidateOnUpdatePage(
|
||||
"s1",
|
||||
"parent-1",
|
||||
"p1",
|
||||
"New Title",
|
||||
undefined as unknown as string,
|
||||
);
|
||||
|
||||
const p1 = sidebarItem("p1");
|
||||
expect(p1.title).toBe("New Title");
|
||||
expect(p1.icon).toBe("📄"); // preserved, not wiped
|
||||
// Sibling untouched.
|
||||
const p2 = sidebarItem("p2");
|
||||
expect(p2.title).toBe("Other");
|
||||
expect(p2.icon).toBe("📁");
|
||||
});
|
||||
|
||||
it("sidebar cache: icon-only event updates icon but PRESERVES the title", () => {
|
||||
seedSidebar();
|
||||
|
||||
invalidateOnUpdatePage(
|
||||
"s1",
|
||||
"parent-1",
|
||||
"p1",
|
||||
undefined as unknown as string,
|
||||
"🚀",
|
||||
);
|
||||
|
||||
const p1 = sidebarItem("p1");
|
||||
expect(p1.icon).toBe("🚀");
|
||||
expect(p1.title).toBe("Old"); // preserved, not wiped
|
||||
});
|
||||
|
||||
it("does not touch a subtree that lacks the updated node", () => {
|
||||
const otherKey = ["page-tree", "unrelated"];
|
||||
const before = [
|
||||
{ id: "x1", title: "X", icon: "❌", spaceId: "s1" } as IPage,
|
||||
];
|
||||
h.qc.setQueryData<IPage[]>(otherKey, before);
|
||||
|
||||
invalidateOnUpdatePage("s1", "parent-1", "p1", "New", "🚀");
|
||||
|
||||
// Same reference back — the subtree without p1 is left as-is.
|
||||
expect(h.qc.getQueryData<IPage[]>(otherKey)).toBe(before);
|
||||
});
|
||||
});
|
||||
@@ -51,6 +51,10 @@ export function usePageQuery(
|
||||
queryFn: () => getPageById(pageInput),
|
||||
enabled: !!pageInput.pageId,
|
||||
staleTime: 5 * 60 * 1000,
|
||||
// Keep the previously-loaded page visible while navigating to a new one
|
||||
// instead of flashing a blank/skeleton frame (the new page's content
|
||||
// streams in when ready). isLoading stays true only for the very first load.
|
||||
placeholderData: keepPreviousData,
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
@@ -66,6 +70,61 @@ export function usePageQuery(
|
||||
return query;
|
||||
}
|
||||
|
||||
/**
|
||||
* A page view that omits the large, frequently-changing `content` field. Every
|
||||
* other field is preserved, so consumers that read only metadata (title, icon,
|
||||
* permissions, id, creator, timestamps, …) keep working unchanged.
|
||||
*/
|
||||
export type IPageMeta = Omit<IPage, "content">;
|
||||
|
||||
function selectPageMeta(page: IPage): IPageMeta {
|
||||
// Drop `content`; react-query's structural sharing (replaceEqualDeep) then
|
||||
// returns the SAME reference whenever the remaining fields are unchanged, so a
|
||||
// pure content churn (typing / debouncedUpdateContent, collab `page.updated`)
|
||||
// no longer changes this slice's identity and its ~13 subscribers don't
|
||||
// re-render on every keystroke wave.
|
||||
const { content: _content, ...meta } = page;
|
||||
return meta as IPageMeta;
|
||||
}
|
||||
|
||||
/**
|
||||
* Metadata-only variant of {@link usePageQuery}. Shares the SAME query cache
|
||||
* entry (`["pages", pageId]`, full object incl. content), but this hook returns
|
||||
* a stable content-less slice so peripheral subscribers stop re-rendering on
|
||||
* every content update. Use it anywhere the full `content` is not read.
|
||||
*/
|
||||
export function usePageMetaQuery(
|
||||
pageInput: Partial<IPageInput>,
|
||||
): UseQueryResult<IPageMeta, Error> {
|
||||
const query = useQuery({
|
||||
queryKey: ["pages", pageInput.pageId],
|
||||
queryFn: () => getPageById(pageInput),
|
||||
enabled: !!pageInput.pageId,
|
||||
staleTime: 5 * 60 * 1000,
|
||||
select: selectPageMeta,
|
||||
// Match usePageQuery: keep the previous page's metadata visible while
|
||||
// navigating so the periphery (header, breadcrumb, …) doesn't flash blank.
|
||||
placeholderData: keepPreviousData,
|
||||
});
|
||||
|
||||
// Mirror usePageQuery's cross-key alias write so a page fetched by one
|
||||
// identifier is also cached under the other. The cache stores the FULL page
|
||||
// (select only narrows what THIS hook returns), so read the full object back
|
||||
// from the cache and alias THAT — never the content-less slice.
|
||||
useEffect(() => {
|
||||
if (!query.data) return;
|
||||
const full = queryClient.getQueryData<IPage>(["pages", pageInput.pageId]);
|
||||
if (!full) return;
|
||||
if (isValidUuid(pageInput.pageId)) {
|
||||
queryClient.setQueryData(["pages", full.slugId], full);
|
||||
} else {
|
||||
queryClient.setQueryData(["pages", full.id], full);
|
||||
}
|
||||
}, [query.data]);
|
||||
|
||||
return query;
|
||||
}
|
||||
|
||||
export function useCreatePageMutation() {
|
||||
const { t } = useTranslation();
|
||||
return useMutation<IPage, Error, Partial<IPageInput>>({
|
||||
@@ -351,6 +410,12 @@ export function useRecentChangesQuery(spaceId?: string) {
|
||||
initialPageParam: undefined as string | undefined,
|
||||
getNextPageParam: (lastPage) =>
|
||||
lastPage.meta.hasNextPage ? lastPage.meta.nextCursor : undefined,
|
||||
// KEEP refetchOnMount:true (against the global default false): recent-changes
|
||||
// IS invalidated on page create/update/move/delete, but invalidateQueries only
|
||||
// marks an UNMOUNTED query stale — it doesn't refetch it. The widget isn't
|
||||
// always mounted, so an event that lands while it's unmounted leaves it stale,
|
||||
// and the global refetchOnMount:false would not re-fetch on remount. The mount
|
||||
// refetch closes that gap.
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
@@ -367,6 +432,9 @@ export function useCreatedByQuery(params?: {
|
||||
initialPageParam: undefined as string | undefined,
|
||||
getNextPageParam: (lastPage) =>
|
||||
lastPage.meta.hasNextPage ? lastPage.meta.nextCursor : undefined,
|
||||
// KEEP refetchOnMount:true: the "created-by" key is never invalidated (no
|
||||
// socket/mutation path), so the mount refetch is its ONLY freshness mechanism
|
||||
// — without it the list shows stale cache on navigation.
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
@@ -380,8 +448,14 @@ export function useDeletedPagesQuery(
|
||||
queryFn: () => getDeletedPages(spaceId, params),
|
||||
enabled: !!spaceId,
|
||||
placeholderData: keepPreviousData,
|
||||
refetchOnMount: true,
|
||||
staleTime: 0,
|
||||
// KEEP refetchOnMount:true: ["trash-list"] IS invalidated by the
|
||||
// move-to-trash / delete / restore mutations, but invalidateQueries only marks
|
||||
// an unmounted query stale — it doesn't refetch it. The trash panel isn't
|
||||
// usually mounted when a page is trashed, so on opening it the global
|
||||
// refetchOnMount:false would show a stale list; the mount refetch closes that.
|
||||
// (Do NOT remove the three trash-list invalidations — they are not dead code.)
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -516,7 +590,35 @@ export function invalidateOnUpdatePage(
|
||||
title: string,
|
||||
icon: string,
|
||||
) {
|
||||
invalidatePageTree();
|
||||
// Scoped page-tree refresh (was a blanket `invalidatePageTree()`): this is the
|
||||
// FIELD-only update path (title/icon — no structural change), and the sidebar
|
||||
// tree is already updated pointwise (applyUpdateOne / optimistic setData) plus
|
||||
// via the sidebar-pages cache below. Invalidating ALL ["page-tree"] queries
|
||||
// here refetched every open recursive subpages-embed block on each
|
||||
// rename/icon-change — pure duplicate work. Instead patch just the affected
|
||||
// node IN PLACE in every cached embed subtree: same visible result, no network
|
||||
// churn, no full embed-tree rebuild. Structural events (create/move/delete)
|
||||
// keep the blanket invalidate in their own helpers.
|
||||
const pageTreeMatches = queryClient.getQueriesData<IPage[]>({
|
||||
queryKey: ["page-tree"],
|
||||
});
|
||||
pageTreeMatches.forEach(([key, items]) => {
|
||||
if (!items || !items.some((p) => p.id === id)) return;
|
||||
queryClient.setQueryData<IPage[]>(key, (old) =>
|
||||
old?.map((p) =>
|
||||
p.id === id
|
||||
? {
|
||||
...p,
|
||||
// Guard undefined so a title-only event can't wipe the icon (and
|
||||
// vice versa) in the embed cache.
|
||||
...(title !== undefined ? { title } : {}),
|
||||
...(icon !== undefined ? { icon } : {}),
|
||||
}
|
||||
: p,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
let queryKey: QueryKey = null;
|
||||
if (parentPageId === null) {
|
||||
queryKey = ["root-sidebar-pages", spaceId];
|
||||
@@ -534,7 +636,14 @@ export function invalidateOnUpdatePage(
|
||||
...page,
|
||||
items: page.items.map((sidebarPage: IPage) =>
|
||||
sidebarPage.id === id
|
||||
? { ...sidebarPage, title: title, icon: icon }
|
||||
? {
|
||||
...sidebarPage,
|
||||
// Guard undefined so a title-only event can't wipe the icon
|
||||
// (and vice versa) in the sidebar-pages cache — mirrors the
|
||||
// embed-cache patch above.
|
||||
...(title !== undefined ? { title } : {}),
|
||||
...(icon !== undefined ? { icon } : {}),
|
||||
}
|
||||
: sidebarPage,
|
||||
),
|
||||
})),
|
||||
|
||||
@@ -7,7 +7,7 @@ import { useRestorePageModal } from "@/features/page/hooks/use-restore-page-moda
|
||||
import { useDeletePageModal } from "@/features/page/hooks/use-delete-page-modal.tsx";
|
||||
import {
|
||||
useDeletePageMutation,
|
||||
usePageQuery,
|
||||
usePageMetaQuery,
|
||||
useRestorePageMutation,
|
||||
} from "@/features/page/queries/page-query.ts";
|
||||
import { getSpaceUrl } from "@/lib/config.ts";
|
||||
@@ -25,7 +25,7 @@ type DeletedPageBannerProps = {
|
||||
export function DeletedPageBanner({ slugId }: DeletedPageBannerProps) {
|
||||
const { t } = useTranslation();
|
||||
const navigate = useNavigate();
|
||||
const { data: page } = usePageQuery({ pageId: slugId });
|
||||
const { data: page } = usePageMetaQuery({ pageId: slugId });
|
||||
const { data: space } = useGetSpaceBySlugQuery(page?.space?.slug);
|
||||
const spaceAbility = useSpaceAbility(space?.membership?.permissions);
|
||||
const deletedTimeAgo = useTimeAgo(page?.deletedAt);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useAtom } from "jotai";
|
||||
import { useSetAtom, useStore } from "jotai";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useParams } from "react-router-dom";
|
||||
import { ActionIcon, Menu, rem } from "@mantine/core";
|
||||
@@ -52,7 +52,11 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) {
|
||||
const clipboard = useClipboard({ timeout: 500 });
|
||||
const { spaceSlug } = useParams();
|
||||
const { handleDelete } = useTreeMutation(node.spaceId);
|
||||
const [data, setData] = useAtom(treeDataAtom);
|
||||
// Setter-only: the tree value is read only imperatively inside the duplicate
|
||||
// handler (via `store` below), never at render, so useSetAtom avoids
|
||||
// re-rendering every row's NodeMenu on any tree event.
|
||||
const setData = useSetAtom(treeDataAtom);
|
||||
const store = useStore();
|
||||
const emit = useQueryEmit();
|
||||
const [exportOpened, { open: openExportModal, close: closeExportModal }] =
|
||||
useDisclosure(false);
|
||||
@@ -125,8 +129,8 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) {
|
||||
try {
|
||||
const duplicatedPage = await duplicatePage({ pageId: node.id });
|
||||
|
||||
// figure out parent + insertion index
|
||||
const siblings = treeModel.siblingsOf(data, node.id);
|
||||
// figure out parent + insertion index (read the live tree imperatively)
|
||||
const siblings = treeModel.siblingsOf(store.get(treeDataAtom), node.id);
|
||||
const parentId = siblings?.parentId ?? null;
|
||||
const currentIndex = siblings?.index ?? 0;
|
||||
const newIndex = currentIndex + 1;
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useRef } from "react";
|
||||
import { Link, useParams } from "react-router-dom";
|
||||
import { useAtom } from "jotai";
|
||||
import { useAtom, useSetAtom } from "jotai";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { ActionIcon, rem, Tooltip } from "@mantine/core";
|
||||
import {
|
||||
@@ -51,7 +51,11 @@ export function SpaceTreeRow({
|
||||
const { t } = useTranslation();
|
||||
const { spaceSlug } = useParams();
|
||||
const updatePageMutation = useUpdatePageMutation();
|
||||
const [, setTreeData] = useAtom(treeDataAtom);
|
||||
// Setter-only: subscribing to the whole treeDataAtom (via useAtom) re-rendered
|
||||
// every virtualized row on any tree event, bypassing the DocTreeRow memo. This
|
||||
// row never reads the tree value, only writes it, so useSetAtom avoids the
|
||||
// value subscription.
|
||||
const setTreeData = useSetAtom(treeDataAtom);
|
||||
const emit = useQueryEmit();
|
||||
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
const [mobileSidebarOpened] = useAtom(mobileSidebarAtom);
|
||||
|
||||
@@ -35,6 +35,7 @@ vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
isFetching: false,
|
||||
}),
|
||||
usePageQuery: () => ({ data: undefined }),
|
||||
usePageMetaQuery: () => ({ data: undefined }),
|
||||
fetchAllAncestorChildren: (...args: unknown[]) =>
|
||||
fetchAllAncestorChildrenMock(...args),
|
||||
}));
|
||||
|
||||
@@ -26,6 +26,7 @@ vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
isFetching: false,
|
||||
}),
|
||||
usePageQuery: () => ({ data: undefined }),
|
||||
usePageMetaQuery: () => ({ data: undefined }),
|
||||
fetchAllAncestorChildren: vi.fn(),
|
||||
}));
|
||||
|
||||
|
||||
@@ -15,7 +15,7 @@ import { notifications } from "@mantine/notifications";
|
||||
import {
|
||||
fetchAllAncestorChildren,
|
||||
useGetRootSidebarPagesQuery,
|
||||
usePageQuery,
|
||||
usePageMetaQuery,
|
||||
} from "@/features/page/queries/page-query.ts";
|
||||
import classes from "@/features/page/tree/styles/tree.module.css";
|
||||
import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
|
||||
@@ -76,7 +76,7 @@ const SpaceTree = forwardRef<SpaceTreeApi, SpaceTreeProps>(function SpaceTree(
|
||||
const [isDataLoaded, setIsDataLoaded] = useState(false);
|
||||
const spaceIdRef = useRef(spaceId);
|
||||
spaceIdRef.current = spaceId;
|
||||
const { data: currentPage } = usePageQuery({
|
||||
const { data: currentPage } = usePageMetaQuery({
|
||||
pageId: extractPageSlugId(pageSlug),
|
||||
});
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { useCallback } from "react";
|
||||
import { useAtom, useSetAtom, useStore } from "jotai";
|
||||
import { useSetAtom, useStore } from "jotai";
|
||||
import { notifications } from "@mantine/notifications";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useNavigate, useParams } from "react-router-dom";
|
||||
@@ -34,7 +34,10 @@ export type UseTreeMutation = {
|
||||
|
||||
export function useTreeMutation(spaceId: string): UseTreeMutation {
|
||||
const { t } = useTranslation();
|
||||
const [, setData] = useAtom(treeDataAtom);
|
||||
// Setter-only: this hook never reads the tree reactively (handlers read the
|
||||
// live value imperatively via `store` below), so useSetAtom avoids
|
||||
// re-rendering SpaceSidebar on every tree event.
|
||||
const setData = useSetAtom(treeDataAtom);
|
||||
// `store` reads the *current* treeDataAtom imperatively in handlers — avoids
|
||||
// stale-closure issues when the caller updates the tree (e.g. lazy-load
|
||||
// children) and then immediately invokes a handler.
|
||||
|
||||
@@ -28,6 +28,7 @@ vi.mock("@/features/share/queries/share-query.ts", () => ({
|
||||
|
||||
vi.mock("@/features/page/queries/page-query.ts", () => ({
|
||||
usePageQuery: () => ({ data: { id: "page-1", title: "Doc" } }),
|
||||
usePageMetaQuery: () => ({ data: { id: "page-1", title: "Doc" } }),
|
||||
}));
|
||||
|
||||
vi.mock("@/features/space/queries/space-query.ts", () => ({
|
||||
|
||||
@@ -20,7 +20,7 @@ import {
|
||||
import { Link, useParams } from "react-router-dom";
|
||||
import { extractPageSlugId, getPageIcon } from "@/lib";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query.ts";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query.ts";
|
||||
import CopyTextButton from "@/components/common/copy.tsx";
|
||||
import { getAppUrl } from "@/lib/config.ts";
|
||||
import { buildPageUrl } from "@/features/page/page.utils.ts";
|
||||
@@ -37,7 +37,7 @@ export default function ShareModal({ readOnly }: ShareModalProps) {
|
||||
const { t } = useTranslation();
|
||||
const { pageSlug } = useParams();
|
||||
const pageSlugId = extractPageSlugId(pageSlug);
|
||||
const { data: page } = usePageQuery({ pageId: pageSlugId });
|
||||
const { data: page } = usePageMetaQuery({ pageId: pageSlugId });
|
||||
const pageId = page?.id;
|
||||
const { data: share } = useShareForPageQuery(pageId);
|
||||
const { spaceSlug } = useParams();
|
||||
|
||||
@@ -38,6 +38,11 @@ export function useGetSpacesQuery(
|
||||
queryKey: ["spaces", params],
|
||||
queryFn: () => getSpaces(params),
|
||||
placeholderData: keepPreviousData,
|
||||
// KEEP refetchOnMount:true (against the global default false): the ["spaces"]
|
||||
// key is invalidated only by same-tab mutations (no socket path), so a
|
||||
// cross-actor change — an admin adding/removing THIS user from a space — has
|
||||
// no local mutation or socket event and would leave the space list stale until
|
||||
// a hard reload. The mount refetch is its only cross-actor freshness path.
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -16,7 +16,6 @@ export function useWatchedSpaceIds(): Set<string> {
|
||||
const { data } = useQuery({
|
||||
queryKey: [WATCHED_SPACE_IDS_KEY],
|
||||
queryFn: () => getWatchedSpaceIds(),
|
||||
refetchOnMount: true,
|
||||
});
|
||||
|
||||
const items = data?.items;
|
||||
|
||||
@@ -19,7 +19,11 @@ export const useQuerySubscription = () => {
|
||||
const [socket] = useAtom(socketAtom);
|
||||
|
||||
React.useEffect(() => {
|
||||
socket?.on("message", (event) => {
|
||||
if (!socket) return;
|
||||
// Named handler + off() cleanup (mirrors use-notification-socket). Without
|
||||
// cleanup, every socket recreation / effect re-run stacked another listener,
|
||||
// so a single broadcast fired duplicated invalidateQueries / setQueryData.
|
||||
const handleMessage = (event) => {
|
||||
const data: WebSocketEvent = event;
|
||||
|
||||
let entity = null;
|
||||
@@ -163,6 +167,11 @@ export const useQuerySubscription = () => {
|
||||
});
|
||||
break;
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
socket.on("message", handleMessage);
|
||||
return () => {
|
||||
socket.off("message", handleMessage);
|
||||
};
|
||||
}, [queryClient, socket]);
|
||||
};
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useEffect } from "react";
|
||||
import { socketAtom } from "@/features/websocket/atoms/socket-atom.ts";
|
||||
import { useAtom } from "jotai";
|
||||
import { useAtom, useSetAtom } from "jotai";
|
||||
import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts";
|
||||
import { WebSocketEvent } from "@/features/websocket/types";
|
||||
import { SpaceTreeNode } from "@/features/page/tree/types.ts";
|
||||
@@ -16,7 +16,10 @@ import localEmitter from "@/lib/local-emitter.ts";
|
||||
|
||||
export const useTreeSocket = () => {
|
||||
const [socket] = useAtom(socketAtom);
|
||||
const [, setTreeData] = useAtom(treeDataAtom);
|
||||
// Setter-only: this hook writes the tree from socket events but never reads it
|
||||
// reactively, so useSetAtom avoids re-rendering UserProvider (its host) on
|
||||
// every tree event.
|
||||
const setTreeData = useSetAtom(treeDataAtom);
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
useEffect(() => {
|
||||
@@ -37,7 +40,11 @@ export const useTreeSocket = () => {
|
||||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
socket?.on("message", (event: WebSocketEvent) => {
|
||||
if (!socket) return;
|
||||
// Named handler + off() cleanup (mirrors use-notification-socket). Without
|
||||
// cleanup, every socket recreation / effect re-run stacked another listener,
|
||||
// so a single broadcast fired duplicated tree walks after each reconnect.
|
||||
const handleMessage = (event: WebSocketEvent) => {
|
||||
switch (event.operation) {
|
||||
case "updateOne":
|
||||
if (event.entity[0] === "pages") {
|
||||
@@ -64,6 +71,11 @@ export const useTreeSocket = () => {
|
||||
});
|
||||
break;
|
||||
}
|
||||
});
|
||||
}, [socket]);
|
||||
};
|
||||
|
||||
socket.on("message", handleMessage);
|
||||
return () => {
|
||||
socket.off("message", handleMessage);
|
||||
};
|
||||
}, [socket, queryClient, setTreeData]);
|
||||
};
|
||||
|
||||
@@ -243,6 +243,5 @@ export function useAppVersion(
|
||||
queryFn: () => getAppVersion(),
|
||||
staleTime: 60 * 60 * 1000, // 1 hr
|
||||
enabled: isEnabled,
|
||||
refetchOnMount: true,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useNavigate, useParams } from "react-router-dom";
|
||||
import { useEffect } from "react";
|
||||
import { usePageQuery } from "@/features/page/queries/page-query";
|
||||
import { usePageMetaQuery } from "@/features/page/queries/page-query";
|
||||
import { buildPageUrl } from "@/features/page/page.utils.ts";
|
||||
import { extractPageSlugId } from "@/lib";
|
||||
import { Error404 } from "@/components/ui/error-404.tsx";
|
||||
@@ -11,7 +11,7 @@ export default function PageRedirect() {
|
||||
data: page,
|
||||
isLoading: pageIsLoading,
|
||||
isError,
|
||||
} = usePageQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
} = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug) });
|
||||
const navigate = useNavigate();
|
||||
|
||||
useEffect(() => {
|
||||
|
||||
@@ -10,7 +10,7 @@ import { useTranslation } from "react-i18next";
|
||||
import React from "react";
|
||||
import { EmptyState } from "@/components/ui/empty-state.tsx";
|
||||
import { IconAlertTriangle, IconFileOff } from "@tabler/icons-react";
|
||||
import { Button } from "@mantine/core";
|
||||
import { Button, Skeleton } from "@mantine/core";
|
||||
import { Link } from "react-router-dom";
|
||||
import { ErrorBoundary } from "react-error-boundary";
|
||||
const MemoizedFullEditor = React.memo(FullEditor);
|
||||
@@ -58,7 +58,7 @@ function PageContent({ pageSlug }: { pageSlug: string | undefined }) {
|
||||
(space?.settings?.comments?.allowViewerComments === true);
|
||||
|
||||
if (isLoading) {
|
||||
return <></>;
|
||||
return <PageSkeleton />;
|
||||
}
|
||||
|
||||
if (isError || !page) {
|
||||
@@ -87,7 +87,7 @@ function PageContent({ pageSlug }: { pageSlug: string | undefined }) {
|
||||
}
|
||||
|
||||
if (!space) {
|
||||
return <></>;
|
||||
return <PageSkeleton />;
|
||||
}
|
||||
|
||||
return (
|
||||
@@ -116,3 +116,18 @@ function PageContent({ pageSlug }: { pageSlug: string | undefined }) {
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
// Lightweight loading placeholder shown instead of a blank fragment while the
|
||||
// page (or its space) is loading, so navigation into a not-yet-cached page no
|
||||
// longer flashes empty. Approximates the title + first content lines.
|
||||
function PageSkeleton() {
|
||||
return (
|
||||
<div>
|
||||
<Skeleton height={34} width="45%" mt="xl" radius="sm" />
|
||||
<Skeleton height={16} mt="xl" radius="sm" />
|
||||
<Skeleton height={16} mt="sm" radius="sm" />
|
||||
<Skeleton height={16} mt="sm" width="85%" radius="sm" />
|
||||
<Skeleton height={16} mt="sm" width="70%" radius="sm" />
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
/**
|
||||
* gitmost #401 — regression test for the connect-vs-unload race in
|
||||
* @hocuspocus/server 3.4.4 (patched via patches/@hocuspocus__server@3.4.4.patch).
|
||||
*
|
||||
* The race (unpatched): when the last client disconnects, storeDocumentHooks'
|
||||
* `finally` schedules an async `unloadDocument`. That unload runs its
|
||||
* `beforeUnloadDocument` hooks asynchronously and, meanwhile, records an
|
||||
* in-flight promise in `this.unloadingDocuments`. In the original 3.4.4
|
||||
* `createDocument`, a NEW connection arriving in that window falls straight
|
||||
* through to the `loadingDocuments`/`documents` checks — it never consults
|
||||
* `unloadingDocuments`. So the new connection can start loading (or reuse) a
|
||||
* document while the old instance is still being torn down; the re-check inside
|
||||
* unload (`shouldUnloadDocument`, which sees 0 connections because async auth
|
||||
* hooks have not registered the new connection yet) then deletes/destroys the
|
||||
* doc out from under the freshly-connected client → orphaned Document → later
|
||||
* redis-sync takes the "doc not loaded" path → sync never completes → the
|
||||
* provider hangs until its ~25s timeout.
|
||||
*
|
||||
* The patch: `createDocument` first awaits any in-flight
|
||||
* `unloadingDocuments.get(name)` before proceeding. Once that settles, the
|
||||
* decision is deterministic — either the doc was fully unloaded (gone from
|
||||
* `documents`, so a clean fresh load) or the unload aborted (healthy doc still
|
||||
* in `documents`, reused). The new connection can never hand-shake onto an
|
||||
* about-to-be-destroyed Document.
|
||||
*
|
||||
* These tests exercise the REAL patched `Hocuspocus.createDocument` (the class
|
||||
* is directly constructible) by seeding `unloadingDocuments` with a controllable
|
||||
* in-flight unload and observing that createDocument waits for it.
|
||||
*/
|
||||
import { Hocuspocus } from '@hocuspocus/server';
|
||||
|
||||
// A promise we can resolve on demand, to model an unload that is mid-flight.
|
||||
function deferred<T = void>() {
|
||||
let resolve!: (v: T) => void;
|
||||
const promise = new Promise<T>((r) => {
|
||||
resolve = r;
|
||||
});
|
||||
return { promise, resolve };
|
||||
}
|
||||
|
||||
describe('gitmost #401 — hocuspocus createDocument awaits in-flight unload', () => {
|
||||
it('does NOT start loading a new doc until the in-flight unload settles, then loads fresh', async () => {
|
||||
const hp = new Hocuspocus();
|
||||
const name = 'page.race';
|
||||
|
||||
// Observe loadDocument: on the unpatched code it is invoked synchronously
|
||||
// within createDocument (before the unload settles); on the patched code it
|
||||
// must be deferred until unloadingDocuments resolves.
|
||||
const freshDoc = { name, __fresh: true } as any;
|
||||
const loadSpy = jest
|
||||
.spyOn(hp as any, 'loadDocument')
|
||||
.mockResolvedValue(freshDoc);
|
||||
|
||||
// Model an unload in progress: an entry sits in unloadingDocuments and, when
|
||||
// it completes, it removes the doc from `documents` (a real full unload).
|
||||
const unload = deferred();
|
||||
(hp as any).documents.set(name, { name, __dying: true });
|
||||
(hp as any).unloadingDocuments.set(
|
||||
name,
|
||||
unload.promise.then(() => {
|
||||
(hp as any).documents.delete(name);
|
||||
}),
|
||||
);
|
||||
|
||||
// Kick off a new connection's createDocument but do not await it yet.
|
||||
const createPromise = (hp as any).createDocument(
|
||||
name,
|
||||
{},
|
||||
'socket-1',
|
||||
{ isAuthenticated: true, readOnly: false },
|
||||
{},
|
||||
);
|
||||
|
||||
// Let all currently-schedulable microtasks run. The patched createDocument is
|
||||
// now parked on `await unloadingDocuments.get(name)`, so loadDocument must
|
||||
// NOT have been called yet, and it must NOT have returned the dying doc.
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
expect(loadSpy).not.toHaveBeenCalled();
|
||||
|
||||
// The unload completes (doc removed from `documents`).
|
||||
unload.resolve();
|
||||
|
||||
// createDocument now proceeds: sees no existing doc → fresh load.
|
||||
const doc = await createPromise;
|
||||
expect(loadSpy).toHaveBeenCalledTimes(1);
|
||||
expect(doc).toBe(freshDoc);
|
||||
// The freshly-loaded doc is the one registered — never the dying instance.
|
||||
expect((hp as any).documents.get(name)).toBe(freshDoc);
|
||||
});
|
||||
|
||||
it('reuses the live doc when the in-flight unload aborts (doc left in documents)', async () => {
|
||||
const hp = new Hocuspocus();
|
||||
const name = 'page.abort';
|
||||
|
||||
const loadSpy = jest.spyOn(hp as any, 'loadDocument');
|
||||
|
||||
// Model an unload that ABORTS (e.g. a new connection reappeared before the
|
||||
// sync re-check): it settles WITHOUT deleting the doc from `documents`.
|
||||
const unload = deferred();
|
||||
const liveDoc = { name, __live: true } as any;
|
||||
(hp as any).documents.set(name, liveDoc);
|
||||
(hp as any).unloadingDocuments.set(name, unload.promise); // no-op unload
|
||||
|
||||
const createPromise = (hp as any).createDocument(
|
||||
name,
|
||||
{},
|
||||
'socket-2',
|
||||
{ isAuthenticated: true, readOnly: false },
|
||||
{},
|
||||
);
|
||||
|
||||
unload.resolve();
|
||||
const doc = await createPromise;
|
||||
|
||||
// The still-present live doc is reused; no fresh load happened.
|
||||
expect(doc).toBe(liveDoc);
|
||||
expect(loadSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('no in-flight unload → behaves normally (fresh load)', async () => {
|
||||
const hp = new Hocuspocus();
|
||||
const name = 'page.normal';
|
||||
const freshDoc = { name } as any;
|
||||
const loadSpy = jest
|
||||
.spyOn(hp as any, 'loadDocument')
|
||||
.mockResolvedValue(freshDoc);
|
||||
|
||||
const doc = await (hp as any).createDocument(
|
||||
name,
|
||||
{},
|
||||
'socket-3',
|
||||
{ isAuthenticated: true, readOnly: false },
|
||||
{},
|
||||
);
|
||||
|
||||
expect(loadSpy).toHaveBeenCalledTimes(1);
|
||||
expect(doc).toBe(freshDoc);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,130 @@
|
||||
/**
|
||||
* gitmost #401 fix 2 — onLoadDocument applies the DB state directly into the
|
||||
* hook's target document and returns undefined (instead of building a NEW Y.Doc
|
||||
* and returning it, which made hocuspocus re-encode+apply the whole state a
|
||||
* SECOND time on every cold load).
|
||||
*
|
||||
* These tests assert:
|
||||
* - the hook mutates `data.document` in place so its content equals the DB doc,
|
||||
* - onLoadDocument returns undefined (so hocuspocus keeps the mutated doc and
|
||||
* does NOT run its own applyUpdate(encodeStateAsUpdate(...)) merge),
|
||||
* - both the raw-ydoc branch and the json→ydoc conversion branch behave so.
|
||||
*
|
||||
* Returning undefined is the observable signal that the double-encode is gone
|
||||
* (the old code returned a new Y.Doc, which made hocuspocus re-encode+apply the
|
||||
* state a second time); we assert that contract rather than counting internal
|
||||
* encode calls, which is brittle given the encodes inside toYdoc and the test's
|
||||
* own `expected` fixtures.
|
||||
*/
|
||||
import * as Y from 'yjs';
|
||||
import { Document } from '@hocuspocus/server';
|
||||
import { TiptapTransformer } from '@hocuspocus/transformer';
|
||||
import { PersistenceExtension } from './persistence.extension';
|
||||
import { tiptapExtensions } from '../collaboration.util';
|
||||
|
||||
// A fresh hocuspocus Document (extends Y.Doc, adds isEmpty()) as hocuspocus
|
||||
// hands to onLoadDocument on a cold load.
|
||||
const freshDoc = () => new Document(`page.${PAGE_ID}`, {});
|
||||
|
||||
const PAGE_ID = '550e8400-e29b-41d4-a716-446655440000';
|
||||
|
||||
const doc = (text: string) => ({
|
||||
type: 'doc',
|
||||
content: [{ type: 'paragraph', content: [{ type: 'text', text }] }],
|
||||
});
|
||||
|
||||
const jsonOf = (ydoc: Y.Doc) =>
|
||||
TiptapTransformer.fromYdoc(ydoc, 'default');
|
||||
|
||||
describe('PersistenceExtension.onLoadDocument — #401 fix 2 (apply-into-hook-doc)', () => {
|
||||
let ext: PersistenceExtension;
|
||||
let pageRepo: { findById: jest.Mock };
|
||||
|
||||
beforeEach(() => {
|
||||
pageRepo = { findById: jest.fn() };
|
||||
ext = new PersistenceExtension(
|
||||
pageRepo as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
{} as any,
|
||||
);
|
||||
jest.spyOn(ext['logger'], 'debug').mockImplementation(() => undefined);
|
||||
jest.spyOn(ext['logger'], 'warn').mockImplementation(() => undefined);
|
||||
});
|
||||
|
||||
const load = (document: Document) =>
|
||||
ext.onLoadDocument({ documentName: `page.${PAGE_ID}`, document } as any);
|
||||
|
||||
it('raw ydoc branch: mutates the hook doc to the DB state and returns undefined', async () => {
|
||||
// Source doc representing the persisted ydoc state.
|
||||
const source = TiptapTransformer.toYdoc(
|
||||
doc('DB CONTENT'),
|
||||
'default',
|
||||
tiptapExtensions,
|
||||
);
|
||||
const dbState = Buffer.from(Y.encodeStateAsUpdate(source));
|
||||
pageRepo.findById.mockResolvedValue({ id: PAGE_ID, ydoc: dbState });
|
||||
|
||||
// The hook target is a fresh empty doc (as hocuspocus supplies on cold load).
|
||||
const target = freshDoc();
|
||||
const result = await load(target);
|
||||
|
||||
// Return undefined so hocuspocus keeps `target` as-is (no second merge).
|
||||
expect(result).toBeUndefined();
|
||||
// The hook document now carries the DB content.
|
||||
expect(jsonOf(target)).toEqual(jsonOf(source));
|
||||
});
|
||||
|
||||
it('json→ydoc branch: converts page.content into the hook doc and returns undefined', async () => {
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
id: PAGE_ID,
|
||||
ydoc: null,
|
||||
content: doc('JSON CONTENT'),
|
||||
});
|
||||
|
||||
const target = freshDoc();
|
||||
const result = await load(target);
|
||||
|
||||
// Returning undefined is what keeps hocuspocus from re-encoding+applying the
|
||||
// state a second time (the old code returned the doc, forcing that extra
|
||||
// encode). We assert the observable contract here — the return value and the
|
||||
// resulting content — rather than counting internal encode calls, which is
|
||||
// brittle: toYdoc and the `expected` build below both encode too.
|
||||
expect(result).toBeUndefined();
|
||||
|
||||
// The converted content landed in the hook document.
|
||||
const expected = TiptapTransformer.toYdoc(
|
||||
doc('JSON CONTENT'),
|
||||
'default',
|
||||
tiptapExtensions,
|
||||
);
|
||||
expect(jsonOf(target)).toEqual(jsonOf(expected));
|
||||
});
|
||||
|
||||
it('live doc already non-empty: early return, no DB read', async () => {
|
||||
// A hocuspocus Document carrying live content (isEmpty('default') === false).
|
||||
const target = freshDoc();
|
||||
const live = TiptapTransformer.toYdoc(
|
||||
doc('LIVE'),
|
||||
'default',
|
||||
tiptapExtensions,
|
||||
);
|
||||
Y.applyUpdate(target, Y.encodeStateAsUpdate(live));
|
||||
|
||||
const result = await load(target);
|
||||
expect(result).toBeUndefined();
|
||||
expect(pageRepo.findById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('no persisted state: leaves the fresh empty doc untouched, returns undefined', async () => {
|
||||
pageRepo.findById.mockResolvedValue({ id: PAGE_ID, ydoc: null, content: null });
|
||||
const target = freshDoc();
|
||||
const result = await load(target);
|
||||
expect(result).toBeUndefined();
|
||||
expect(target.isEmpty('default')).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -171,15 +171,21 @@ export class PersistenceExtension implements Extension {
|
||||
return;
|
||||
}
|
||||
|
||||
// #401 fix 2 — apply the DB state DIRECTLY into the hook's target document
|
||||
// (`document` === `data.document`) and return undefined. When onLoadDocument
|
||||
// returns undefined, hocuspocus keeps the mutated hook document as-is; only
|
||||
// when the hook RETURNS a Y.Doc does hocuspocus re-`applyUpdate(document,
|
||||
// encodeStateAsUpdate(returned))` — a second full encode+apply of the whole
|
||||
// (e.g. 315KB) state on every cold load. Mutating in place performs a single
|
||||
// apply and avoids the throwaway `new Y.Doc()` allocation.
|
||||
if (page.ydoc) {
|
||||
this.logger.debug(`ydoc loaded from db: ${pageId}`);
|
||||
|
||||
const doc = new Y.Doc();
|
||||
const dbState = new Uint8Array(page.ydoc);
|
||||
|
||||
Y.applyUpdate(doc, dbState);
|
||||
Y.applyUpdate(document, dbState);
|
||||
observeCollabLoad(dbState.length, (performance.now() - startedAt) / 1000);
|
||||
return doc;
|
||||
return;
|
||||
}
|
||||
|
||||
// if no ydoc state in db convert json in page.content to Ydoc.
|
||||
@@ -192,18 +198,23 @@ export class PersistenceExtension implements Extension {
|
||||
tiptapExtensions,
|
||||
);
|
||||
|
||||
// Reuse this single encode for the size label (do NOT add a second one).
|
||||
// Encode the converted doc ONCE, reuse the bytes for both the size label
|
||||
// and the single apply into the hook document (previously this encode's
|
||||
// result was returned and hocuspocus re-encoded+applied it a second time).
|
||||
const encoded = Y.encodeStateAsUpdate(ydoc);
|
||||
Y.applyUpdate(document, encoded);
|
||||
observeCollabLoad(
|
||||
encoded.byteLength,
|
||||
(performance.now() - startedAt) / 1000,
|
||||
);
|
||||
return ydoc;
|
||||
return;
|
||||
}
|
||||
|
||||
// No persisted state: the hook document is already a fresh empty Y.Doc, so
|
||||
// leave it untouched and return undefined (no re-encode of an empty doc).
|
||||
this.logger.debug(`creating fresh ydoc: ${pageId}`);
|
||||
observeCollabLoad(0, (performance.now() - startedAt) / 1000);
|
||||
return new Y.Doc();
|
||||
return;
|
||||
}
|
||||
|
||||
async onStoreDocument(data: onStoreDocumentPayload) {
|
||||
|
||||
@@ -17,10 +17,24 @@ import { Injectable, Logger, OnModuleDestroy } from '@nestjs/common';
|
||||
/** How long a finished entry is retained for late attach (replay + immediate end). */
|
||||
export const RUN_STREAM_RETAIN_FINISHED_MS = 30_000;
|
||||
|
||||
/** Per-run replay buffer cap. Past this the buffer is dropped (attach -> 204). */
|
||||
export const RUN_STREAM_MAX_BUFFER_BYTES = 4 * 1024 * 1024;
|
||||
/**
|
||||
* Per-run replay buffer cap. Past this the buffer is dropped (attach -> 204, and
|
||||
* the client falls back to its restore + degraded-poll path, #430).
|
||||
*
|
||||
* Raised from 4MB to 32MB (#430): marathon autonomous runs (11-25 min observed)
|
||||
* stream far more than 4MB of SSE frames, so a live disconnect mid-run would find
|
||||
* an already-overflowed buffer and could only degrade-poll instead of re-attaching
|
||||
* to the live tail. 32MB comfortably covers those runs while staying bounded.
|
||||
*
|
||||
* Memory cost: this is the WORST-CASE retained size PER ACTIVE run (the buffer is
|
||||
* freed on finish + retention, or dropped immediately on overflow). With the small
|
||||
* number of concurrent autonomous runs a single workspace realistically has, 32MB
|
||||
* each is an acceptable ceiling; the overflow->204->degraded-poll fallback remains
|
||||
* the backstop for anything larger, so correctness never depends on this bound.
|
||||
*/
|
||||
export const RUN_STREAM_MAX_BUFFER_BYTES = 32 * 1024 * 1024;
|
||||
|
||||
// 2x the replay cap: a just-written 4MB replay burst alone can never trip the
|
||||
// 2x the replay cap: a just-written full-replay burst alone can never trip the
|
||||
// per-subscriber cap (see controller); only a genuinely stalled socket can.
|
||||
export const SUBSCRIBER_MAX_BUFFERED_BYTES = 2 * RUN_STREAM_MAX_BUFFER_BYTES;
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import {
|
||||
AiChatStreamRegistryService,
|
||||
RUN_STREAM_MAX_BUFFER_BYTES,
|
||||
RUN_STREAM_RETAIN_FINISHED_MS,
|
||||
SUBSCRIBER_MAX_BUFFERED_BYTES,
|
||||
RunStreamCallbacks,
|
||||
} from './ai-chat-stream-registry.service';
|
||||
|
||||
@@ -210,9 +211,10 @@ describe('AiChatStreamRegistryService', () => {
|
||||
const att = (await registry.attach(CHAT, false, undefined, c.cb))!;
|
||||
att.start();
|
||||
|
||||
const oneMb = 'x'.repeat(1024 * 1024);
|
||||
// 5 x 1MB = 5MB > 4MB cap; the 5th frame is the one that crosses.
|
||||
for (let i = 0; i < 5; i++) src.push(oneMb + i);
|
||||
// Cap-relative so it survives a buffer-cap change (#430): a quarter-cap frame
|
||||
// means 5 frames comfortably exceed the replay cap; the last one crosses.
|
||||
const chunk = 'x'.repeat(Math.floor(RUN_STREAM_MAX_BUFFER_BYTES / 4));
|
||||
for (let i = 0; i < 5; i++) src.push(chunk + i);
|
||||
await flush();
|
||||
|
||||
const entry = (registry as any).entries.get(CHAT);
|
||||
@@ -220,7 +222,7 @@ describe('AiChatStreamRegistryService', () => {
|
||||
expect(entry.bytes).toBeGreaterThan(RUN_STREAM_MAX_BUFFER_BYTES);
|
||||
// The live subscriber received ALL 5 frames, including the crossing one.
|
||||
expect(c.frames).toHaveLength(5);
|
||||
expect(c.frames[4]).toBe(oneMb + 4);
|
||||
expect(c.frames[4]).toBe(chunk + 4);
|
||||
|
||||
// A NEW attach after overflow gets null (replay buffer is gone).
|
||||
const c2 = collector();
|
||||
@@ -240,9 +242,11 @@ describe('AiChatStreamRegistryService', () => {
|
||||
const attB = (await registry.attach(CHAT, false, undefined, b.cb))!;
|
||||
attB.start();
|
||||
|
||||
const oneMb = 'x'.repeat(1024 * 1024);
|
||||
// 9 x 1MB = 9MB > 8MB per-subscriber cap; A's pending overflows, B streams live.
|
||||
for (let i = 0; i < 9; i++) src.push(oneMb + i);
|
||||
// Cap-relative so it survives a buffer-cap change (#430): a quarter-of-the-
|
||||
// per-subscriber-cap frame means 5 frames exceed A's paused-pending cap while
|
||||
// B streams every frame live.
|
||||
const chunk = 'x'.repeat(Math.floor(SUBSCRIBER_MAX_BUFFERED_BYTES / 4));
|
||||
for (let i = 0; i < 5; i++) src.push(chunk + i);
|
||||
await flush();
|
||||
|
||||
const entry = (registry as any).entries.get(CHAT);
|
||||
@@ -250,7 +254,7 @@ describe('AiChatStreamRegistryService', () => {
|
||||
expect(entry.subscribers.size).toBe(1);
|
||||
expect(a.frames).toEqual([]); // paused + overflowed: nothing was delivered
|
||||
// B received every frame live (delivery unaffected by A's overflow).
|
||||
expect(b.frames).toHaveLength(9);
|
||||
expect(b.frames).toHaveLength(5);
|
||||
|
||||
// A's start() (arriving late) degrades to an immediate end, not a partial replay.
|
||||
attA.start();
|
||||
|
||||
@@ -148,6 +148,53 @@ describe('assistantParts', () => {
|
||||
expect(toolPart).not.toHaveProperty('output');
|
||||
});
|
||||
|
||||
it('replays the REAL error text for a THROWN tool (tool-error part)', () => {
|
||||
const steps = [
|
||||
{
|
||||
text: '',
|
||||
toolCalls: [
|
||||
{ toolCallId: 'c7', toolName: 'editPageText', input: { id: 'p1' } },
|
||||
],
|
||||
// A thrown tool is a `tool-error` content part; toolResults holds only
|
||||
// successes and stays empty for this call.
|
||||
toolResults: [],
|
||||
content: [
|
||||
{
|
||||
type: 'tool-error',
|
||||
toolCallId: 'c7',
|
||||
toolName: 'editPageText',
|
||||
input: { id: 'p1' },
|
||||
error: new Error('page is locked'),
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
const parts = assistantParts(steps, '') as AnyPart[];
|
||||
const toolPart = parts.find((p) => p.type === 'tool-editPageText');
|
||||
expect(toolPart).toBeDefined();
|
||||
expect(toolPart!.state).toBe('output-error');
|
||||
// The REAL error is replayed, NOT the 'Tool call did not complete.' placeholder.
|
||||
expect(toolPart!.errorText).toBe('page is locked');
|
||||
expect(toolPart).not.toHaveProperty('output');
|
||||
});
|
||||
|
||||
it('keeps the placeholder ONLY for a call with neither result nor tool-error', () => {
|
||||
const steps = [
|
||||
{
|
||||
text: '',
|
||||
toolCalls: [
|
||||
{ toolCallId: 'c8', toolName: 'insertNode', input: { node: {} } },
|
||||
],
|
||||
toolResults: [],
|
||||
content: [], // aborted mid-step: no result AND no tool-error
|
||||
},
|
||||
];
|
||||
const parts = assistantParts(steps, '') as AnyPart[];
|
||||
const toolPart = parts.find((p) => p.type === 'tool-insertNode');
|
||||
expect(toolPart!.state).toBe('output-error');
|
||||
expect(toolPart!.errorText).toBe('Tool call did not complete.');
|
||||
});
|
||||
|
||||
it('skips malformed tool-calls (missing toolName or toolCallId)', () => {
|
||||
const steps = [
|
||||
{
|
||||
@@ -195,6 +242,45 @@ describe('serializeSteps', () => {
|
||||
expect(trace[0]).toEqual({ toolName: 'getPage', input: { id: 'p1' } });
|
||||
expect(trace[1]).toEqual({ toolName: 'getPage', output: { title: 'T' } });
|
||||
});
|
||||
|
||||
it('records a THROWN tool failure (tool-error part) with its error message', () => {
|
||||
const trace = serializeSteps([
|
||||
{
|
||||
toolCalls: [{ toolName: 'editPageText', input: { id: 'p1' } }],
|
||||
toolResults: [],
|
||||
content: [
|
||||
{
|
||||
type: 'tool-error',
|
||||
toolName: 'editPageText',
|
||||
error: new Error('page is locked'),
|
||||
},
|
||||
],
|
||||
},
|
||||
]) as Array<Record<string, unknown>>;
|
||||
// The call element is followed by a paired error element (mirroring how a
|
||||
// successful result is appended), so the failure survives in the trace.
|
||||
expect(trace).toHaveLength(2);
|
||||
expect(trace[0]).toEqual({ toolName: 'editPageText', input: { id: 'p1' } });
|
||||
expect(trace[1]).toEqual({
|
||||
toolName: 'editPageText',
|
||||
error: 'page is locked',
|
||||
});
|
||||
});
|
||||
|
||||
it('truncates a very long tool-error message to the tool-output limit', () => {
|
||||
const long = 'x'.repeat(5000);
|
||||
const trace = serializeSteps([
|
||||
{
|
||||
toolCalls: [{ toolName: 'editPageText', input: {} }],
|
||||
toolResults: [],
|
||||
content: [{ type: 'tool-error', toolName: 'editPageText', error: long }],
|
||||
},
|
||||
]) as Array<Record<string, unknown>>;
|
||||
const errorText = trace[1].error as string;
|
||||
// Truncated (not the full 5000 chars) and carries the omission marker.
|
||||
expect(errorText.length).toBeLessThan(long.length);
|
||||
expect(errorText).toContain('chars omitted');
|
||||
});
|
||||
});
|
||||
|
||||
describe('rowToUiMessage', () => {
|
||||
|
||||
@@ -1637,6 +1637,17 @@ type StepLike = {
|
||||
toolName?: string;
|
||||
output?: unknown;
|
||||
}>;
|
||||
// ai@6.0.134: a tool that THREW surfaces as a `tool-error` content part
|
||||
// ({ type:'tool-error', toolCallId, toolName, input, error }), NOT as a
|
||||
// `toolResults` entry (which holds only successes). Read from here so failed
|
||||
// calls are persisted with their real error instead of being dropped.
|
||||
content?: ReadonlyArray<{
|
||||
type?: string;
|
||||
toolCallId?: string;
|
||||
toolName?: string;
|
||||
input?: unknown;
|
||||
error?: unknown;
|
||||
}>;
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -1739,6 +1750,26 @@ function compactValue(value: unknown, depth: number): unknown {
|
||||
return value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract a bounded string message from a `tool-error` part's `error` field for
|
||||
* persistence and history replay. The field may be an `Error`, a string, or an
|
||||
* arbitrary object, so pull a message robustly. The result is passed through
|
||||
* `compactValue` so a very long error honors the SAME truncation limits the file
|
||||
* already applies to tool outputs (no new limit is introduced here).
|
||||
*/
|
||||
function normalizeToolError(error: unknown): string {
|
||||
const message =
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: typeof error === 'string'
|
||||
? error
|
||||
: error != null &&
|
||||
typeof (error as { message?: unknown }).message === 'string'
|
||||
? (error as { message: string }).message
|
||||
: String(error);
|
||||
return compactValue(message, 0) as string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Rebuild the FULL UIMessage `parts` for an assistant turn from the SDK steps,
|
||||
* so multi-turn history replays prior tool-calls/results to the model (not just
|
||||
@@ -1771,6 +1802,14 @@ export function assistantParts(
|
||||
for (const r of step.toolResults ?? []) {
|
||||
if (r.toolCallId) resultsById.set(r.toolCallId, r.output);
|
||||
}
|
||||
// Index this step's THROWN tool failures (ai@6 `tool-error` content parts)
|
||||
// by tool call id, so a call that failed replays with its real error text.
|
||||
const errorsById = new Map<string, unknown>();
|
||||
for (const part of step.content ?? []) {
|
||||
if (part.type === 'tool-error' && part.toolCallId) {
|
||||
errorsById.set(part.toolCallId, part.error);
|
||||
}
|
||||
}
|
||||
for (const call of step.toolCalls ?? []) {
|
||||
if (!call.toolName || !call.toolCallId) continue;
|
||||
const hasResult = resultsById.has(call.toolCallId);
|
||||
@@ -1783,9 +1822,21 @@ export function assistantParts(
|
||||
input: call.input,
|
||||
output: compactToolOutput(resultsById.get(call.toolCallId)),
|
||||
});
|
||||
} else if (errorsById.has(call.toolCallId)) {
|
||||
// The tool THREW: replay the REAL error so the model on the next turn
|
||||
// knows WHY the call failed (and does not blindly repeat it). An
|
||||
// output-error round-trips through convertToModelMessages as a balanced
|
||||
// tool-call + tool-result, keeping the rebuilt history valid.
|
||||
parts.push({
|
||||
type: `tool-${call.toolName}`,
|
||||
toolCallId: call.toolCallId,
|
||||
state: 'output-error',
|
||||
input: call.input,
|
||||
errorText: normalizeToolError(errorsById.get(call.toolCallId)),
|
||||
});
|
||||
} else {
|
||||
// No paired result (e.g. aborted mid-step). Persisting a bare
|
||||
// tool-call (input-available) would replay as an unpaired call and
|
||||
// No paired result AND no tool-error (e.g. aborted mid-step). Persisting
|
||||
// a bare tool-call (input-available) would replay as an unpaired call and
|
||||
// throw MissingToolResultsError on the next turn (convertToModelMessages
|
||||
// emits no tool-result for it). Emit a SYNTHETIC paired result instead:
|
||||
// an output-error round-trips through convertToModelMessages as a
|
||||
@@ -2021,10 +2072,19 @@ export function serializeSteps(
|
||||
steps: ReadonlyArray<{
|
||||
toolCalls?: ReadonlyArray<{ toolName?: string; input?: unknown }>;
|
||||
toolResults?: ReadonlyArray<{ toolName?: string; output?: unknown }>;
|
||||
content?: ReadonlyArray<{
|
||||
type?: string;
|
||||
toolName?: string;
|
||||
error?: unknown;
|
||||
}>;
|
||||
}>,
|
||||
): unknown {
|
||||
const calls: Array<{ toolName?: string; input?: unknown; output?: unknown }> =
|
||||
[];
|
||||
const calls: Array<{
|
||||
toolName?: string;
|
||||
input?: unknown;
|
||||
output?: unknown;
|
||||
error?: string;
|
||||
}> = [];
|
||||
for (const step of steps ?? []) {
|
||||
for (const call of step.toolCalls ?? []) {
|
||||
calls.push({ toolName: call.toolName, input: call.input });
|
||||
@@ -2032,6 +2092,18 @@ export function serializeSteps(
|
||||
for (const r of step.toolResults ?? []) {
|
||||
calls.push({ toolName: r.toolName, output: compactToolOutput(r.output) });
|
||||
}
|
||||
// ai@6 surfaces a THROWN tool failure as a `tool-error` content part, NOT as
|
||||
// a `toolResults` entry. Record it as its own paired element (mirroring how a
|
||||
// successful result is appended) so the failure and its reason survive in the
|
||||
// trace instead of leaving an orphaned call with no result.
|
||||
for (const part of step.content ?? []) {
|
||||
if (part.type === 'tool-error') {
|
||||
calls.push({
|
||||
toolName: part.toolName,
|
||||
error: normalizeToolError(part.error),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
return calls.length > 0 ? calls : null;
|
||||
}
|
||||
|
||||
+77
-39
@@ -13,12 +13,14 @@ Read the **Gotchas** section before you trust any error count.
|
||||
- Agent chats live in Postgres, DB `docmost`, tables `ai_chat_*`.
|
||||
- Each tool invocation is stored as **two** array elements (a `tool-call` part and
|
||||
a `tool-result` part), so naive counting double-counts.
|
||||
- **A tool that *throws* writes no result part at all.** Its error text is nowhere
|
||||
in the DB — not in `tool_calls`, `content`, or `metadata`. It is shown live in
|
||||
the UI only. So `isError` / `success=false` scans under-report by design.
|
||||
- To find where agents fail you need **three** sources: (1) soft-failure markers in
|
||||
`tool_calls`, (2) the orphan-gap proxy for thrown errors, (3) server logs / the
|
||||
live UI for the actual error text.
|
||||
- **A tool that *throws* writes no result part.** Since the #407 fix its error is
|
||||
persisted as a dedicated `{toolName, error}` element in `tool_calls` (queryable +
|
||||
replayed to the model). **Rows written before #407 still drop it** — the error is
|
||||
nowhere in the DB and shows only in the live UI. So `isError` / `success=false`
|
||||
scans under-report by design, and pre-#407 thrown errors are invisible.
|
||||
- To find where agents fail: (1) soft-failure markers in `tool_calls`, (2) the new
|
||||
`error` field for thrown errors (new rows) / the orphan-gap proxy (old rows),
|
||||
(3) server logs / the live UI for full stack traces beyond the truncated message.
|
||||
|
||||
## Where the data lives
|
||||
|
||||
@@ -61,13 +63,17 @@ index 0: { "toolName": "getPage", "input": { "pageId": "…" } } ← tool-ca
|
||||
index 1: { "toolName": "getPage", "output": { … } } ← tool-result (has output, NO input)
|
||||
```
|
||||
|
||||
The **only** keys that ever appear on an element are `toolName`, `input`, `output`.
|
||||
There is no `state`, no `errorText`, no `type`. Consequences:
|
||||
The keys that appear on an element are `toolName`, `input`, `output`, and — for a
|
||||
**thrown** failure on rows written after the #407 fix — `error` (the tool's error
|
||||
message; see the "Hard failures" section below). There is no `state`, no `errorText`,
|
||||
no `type`. On pre-#407 rows a thrown failure has NO paired result element at all
|
||||
(silent orphan). Consequences:
|
||||
|
||||
1. **Real invocation count = elements that have `output`.** Counting every element
|
||||
double-counts (you get ~2× and a spurious "~50% of every tool has no output").
|
||||
2. **Pairing:** a successful call = a `tool-call` part followed by its `tool-result`
|
||||
part. Both carry `toolName`, so you can group by tool on either.
|
||||
1. **Real invocation count = elements that have `output` or `error`.** Counting every
|
||||
element double-counts (you get ~2× and a spurious "~50% of every tool has no output").
|
||||
2. **Pairing:** a call = a `tool-call` part followed by its result part. A success
|
||||
carries `output`; a thrown failure (post-#407) carries `error` instead. Both carry
|
||||
`toolName`, so you can group by tool on either.
|
||||
|
||||
## The two classes of failure (and which the DB can see)
|
||||
|
||||
@@ -85,25 +91,37 @@ These are visible in the `tool-result` `output`. The marker differs per tool:
|
||||
Note `editPageText` returns `failed: []` on success — filtering on the *presence*
|
||||
of the key gives false positives; filter on **non-empty**.
|
||||
|
||||
### 2. Hard failures — tool THREW → NOT PERSISTED ❌ (the trap)
|
||||
### 2. Hard failures — tool THREW → NOW PERSISTED ✅ (since the #407 fix)
|
||||
|
||||
When a tool throws (the classic one is `patchNode` / `insertNode` / `tableUpdateCell`
|
||||
→ `Failed to encode document to Yjs (fromJSON): Unknown node type: undefined`), the
|
||||
runtime writes **no `tool-result` part**. The orphaned `tool-call` part stays, but
|
||||
the error text is **nowhere in the DB**. It is streamed to the UI live and (until
|
||||
rotation) to server logs — that is it.
|
||||
runtime still writes **no `tool-result` part** — the failure is an ai@6 `tool-error`
|
||||
content part instead. **Since the #407 fix, that error is persisted**: `serializeSteps`
|
||||
appends a dedicated element `{toolName, error: "<message>"}` right after the failed
|
||||
call, mirroring how a successful `{toolName, output}` element is appended. So a thrown
|
||||
error now leaves a queryable `error` field carrying its (truncated) reason, and the
|
||||
same real text is replayed to the model on the next turn (an `output-error` part with
|
||||
the real `errorText`, no longer the `'Tool call did not complete.'` placeholder).
|
||||
|
||||
So any query like `count(*) FILTER (WHERE output.success = false)` will happily
|
||||
return **0** for `patchNode` even when the chat is visibly full of red failures.
|
||||
That is survivorship bias, not reliability.
|
||||
**Cutover caveat — old rows keep the old blind shape.** Rows written **before** this
|
||||
change have the two-part shape (`call` + `output` only) and simply **drop** thrown
|
||||
errors, leaving a silent **orphan** (a `call` with no `output` *and* no `error`). Rows
|
||||
written **after** the fix additionally carry the `error` element. So:
|
||||
|
||||
The only DB-side proxy for a thrown error is an **orphan**: a `tool-call` part with
|
||||
no matching `tool-result`. Caveat: orphans also appear when a run is **aborted**
|
||||
mid-flight (server restart), so a high-volume tool (`createComment`, `searchInPage`,
|
||||
`Search_web_search`) shows orphans from aborts, not from real errors. Treat the
|
||||
orphan gap as an *upper bound* on hard errors, and cross-check the tool: a gap on a
|
||||
structural editor (`patchNode`, `insertNode`, `updatePageJson`, `transformPage`) is
|
||||
almost certainly a thrown Yjs-encode error; a gap on `createComment` is mostly aborts.
|
||||
- **New rows:** query the `error` field directly (see the hard-error query below) — no
|
||||
orphan heuristic needed for thrown failures.
|
||||
- **Old rows (pre-#407):** the only DB-side proxy is still an **orphan**: a `tool-call`
|
||||
part with no matching `tool-result` *and* no `error`. Orphans also appear when a run
|
||||
is **aborted** mid-flight (server restart), so a high-volume tool (`createComment`,
|
||||
`searchInPage`, `Search_web_search`) shows orphans from aborts, not real errors on
|
||||
old rows. Treat the orphan gap as an *upper bound*, and cross-check the tool: a gap on
|
||||
a structural editor (`patchNode`, `insertNode`, `updatePageJson`, `transformPage`) is
|
||||
almost certainly a thrown Yjs-encode error; a gap on `createComment` is mostly aborts.
|
||||
|
||||
A note on the aborted-call fallback: a call with **neither** a result **nor** a
|
||||
`tool-error` (genuinely interrupted mid-step) still replays with the
|
||||
`'Tool call did not complete.'` placeholder and persists as an orphan — that path is
|
||||
unchanged, and is distinct from a real thrown error, which now carries `error`.
|
||||
|
||||
### 3. Run-level failures → `ai_chat_runs`
|
||||
|
||||
@@ -164,14 +182,28 @@ WHERE jsonb_typeof(o->'failed') = 'array'
|
||||
GROUP BY 1 ORDER BY 2 DESC;
|
||||
```
|
||||
|
||||
**Hard-error proxy — orphan gap per tool, WITH a spread column** (call parts minus
|
||||
result parts, plus how many distinct chats the gap is spread across):
|
||||
**Hard errors — persisted `error` field per tool (NEW rows, since #407)** — thrown
|
||||
tool failures now carry their real reason, so query them directly:
|
||||
|
||||
```sql
|
||||
SELECT elem->>'toolName' AS tool, count(*) AS thrown_errors,
|
||||
min(elem->>'error') AS sample_error
|
||||
FROM ai_chat_messages m, jsonb_array_elements(m.tool_calls) elem
|
||||
WHERE jsonb_typeof(m.tool_calls) = 'array' AND elem ? 'error'
|
||||
GROUP BY 1 ORDER BY 2 DESC;
|
||||
```
|
||||
|
||||
**Hard-error proxy for OLD rows (pre-#407) — orphan gap per tool, WITH a spread column**
|
||||
(call parts minus result parts, plus how many distinct chats the gap is spread across).
|
||||
This covers rows written before thrown errors were persisted; on new rows a thrown
|
||||
failure now has its own `error` element (use the query above) and an orphan means only
|
||||
a genuinely aborted mid-step call:
|
||||
|
||||
```sql
|
||||
WITH parts AS (
|
||||
SELECT m.chat_id, elem->>'toolName' AS tool,
|
||||
(elem ? 'input' AND NOT (elem ? 'output')) AS is_call,
|
||||
(elem ? 'output') AS is_result
|
||||
(elem ? 'output' OR elem ? 'error') AS is_result
|
||||
FROM ai_chat_messages m, jsonb_array_elements(m.tool_calls) elem
|
||||
WHERE jsonb_typeof(m.tool_calls) = 'array' AND m.role = 'assistant'
|
||||
),
|
||||
@@ -188,8 +220,12 @@ HAVING sum(gap) FILTER (WHERE gap > 0) > 0
|
||||
ORDER BY missing_results DESC;
|
||||
```
|
||||
|
||||
**`missing_results` mixes thrown errors AND aborted/interrupted runs — you cannot
|
||||
split them from `output` alone** (a positional "what follows the orphan" heuristic
|
||||
The `is_result` predicate counts an `error` element as a paired result too, so on new
|
||||
rows a persisted thrown error no longer inflates the orphan gap; a remaining gap is an
|
||||
aborted/interrupted call.
|
||||
|
||||
**On OLD rows, `missing_results` mixes thrown errors AND aborted/interrupted runs — you
|
||||
cannot split them from `output` alone** (a positional "what follows the orphan" heuristic
|
||||
breaks on parallel tool batches, which persist as `call,call,…,result,result`). Use
|
||||
`chats_spread` to disambiguate:
|
||||
|
||||
@@ -244,18 +280,20 @@ docker compose -p gitmost logs -f --tail=100 # whole stack
|
||||
```
|
||||
|
||||
Logging is `json-file`, `max-size=10m max-file=5` → ~50 MB retained, then rotated,
|
||||
and **wiped on container recreate**. So thrown-tool error text is only reliably
|
||||
caught **in real time** (or in the live chat UI, which renders the failed part with
|
||||
its message). There is no durable, queryable store of hard tool errors today — if you
|
||||
need one, that is a feature to add (persist `output-error` parts, or emit a
|
||||
`tool_calls_total{tool,status}` metric to VictoriaMetrics).
|
||||
and **wiped on container recreate**. Since the #407 fix, thrown-tool error text is
|
||||
**persisted in the `error` field** of `tool_calls` (see the hard-error query above), so
|
||||
you no longer depend on live logs for it. Logs/live UI remain useful for **pre-#407
|
||||
rows** (whose thrown errors were dropped) and for full stack traces beyond the
|
||||
truncated stored message. A per-tool `tool_calls_total{tool,status}` metric to
|
||||
VictoriaMetrics is still a possible future add for aggregate dashboards.
|
||||
|
||||
## Gotchas checklist
|
||||
|
||||
- [ ] Counting every `tool_calls` element → **2× overcount**. Count elements with `output`.
|
||||
- [ ] `isError` / `success=false` ≈ 0 does **not** mean "no errors" — thrown errors aren't persisted.
|
||||
- [ ] Counting every `tool_calls` element → **overcount**. Count `output` elements; add `error` elements for thrown failures (new rows), but don't count both as invocations.
|
||||
- [ ] `isError` / `success=false` ≈ 0 does **not** mean "no errors" — thrown errors are a separate `error` element (new rows) or dropped entirely (pre-#407 rows).
|
||||
- [ ] Thrown errors persist only on rows written **after the #407 fix** — pre-#407 rows still drop them (orphan only). Mind the cutover when trending over time.
|
||||
- [ ] `editPageText.failed` is `[]` on success — test for **non-empty**, not presence.
|
||||
- [ ] Orphan gap mixes thrown errors **and** aborted runs — split by tool before concluding.
|
||||
- [ ] Orphan gap on OLD rows mixes thrown errors **and** aborted runs — split by tool. On NEW rows a thrown error is its own `error` element, so a gap ≈ aborted call.
|
||||
- [ ] `aborted` runs = server restarts, `failed` runs = provider overload — not agent mistakes.
|
||||
- [ ] Never dump a raw `tool_calls` cell — it can be hundreds of KB.
|
||||
- [ ] Logs are ephemeral (≤50 MB, wiped on recreate) — grab hard-error text live.
|
||||
|
||||
+2
-1
@@ -97,7 +97,8 @@
|
||||
"patchedDependencies": {
|
||||
"scimmy@1.3.5": "patches/scimmy@1.3.5.patch",
|
||||
"yjs@13.6.30": "patches/yjs@13.6.30.patch",
|
||||
"ai@6.0.134": "patches/ai@6.0.134.patch"
|
||||
"ai@6.0.134": "patches/ai@6.0.134.patch",
|
||||
"@hocuspocus/server@3.4.4": "patches/@hocuspocus__server@3.4.4.patch"
|
||||
},
|
||||
"overrides": {
|
||||
"prosemirror-changeset": "2.4.0",
|
||||
|
||||
@@ -52,6 +52,7 @@
|
||||
"form-data": "^4.0.0",
|
||||
"jsdom": "^27.4.0",
|
||||
"marked": "^17.0.1",
|
||||
"pako": "^2.0.3",
|
||||
"re2": "^1.21.0",
|
||||
"ws": "^8.19.0",
|
||||
"y-prosemirror": "1.3.7",
|
||||
|
||||
+574
-16
@@ -35,6 +35,7 @@ import {
|
||||
deleteNodeById,
|
||||
assertUnambiguousMatch,
|
||||
insertNodeRelative,
|
||||
blockPlainText,
|
||||
buildOutline,
|
||||
getNodeByRef,
|
||||
readTable,
|
||||
@@ -44,6 +45,15 @@ import {
|
||||
} from "./lib/node-ops.js";
|
||||
import { searchInDoc, SearchOptions } from "./lib/page-search.js";
|
||||
import { withPageLock } from "./lib/page-lock.js";
|
||||
import {
|
||||
prepareModel,
|
||||
decodeDrawioSvg,
|
||||
buildDrawioSvg,
|
||||
mxHash,
|
||||
normalizeXml,
|
||||
countUserCells,
|
||||
} from "./lib/drawio-xml.js";
|
||||
import { renderDiagramShapes } from "./lib/drawio-preview.js";
|
||||
import {
|
||||
applyTextEdits,
|
||||
TextEdit,
|
||||
@@ -54,10 +64,12 @@ import { getCollabToken, performLogin } from "./lib/auth-utils.js";
|
||||
import { diffDocs, summarizeChange } from "./lib/diff.js";
|
||||
import {
|
||||
applyAnchorInDoc,
|
||||
canAnchorInDoc,
|
||||
countAnchorMatches,
|
||||
getAnchoredText,
|
||||
resolveAnchorSelection,
|
||||
normalizeForMatch,
|
||||
} from "./lib/comment-anchor.js";
|
||||
import { closestBlockHint } from "./lib/text-normalize.js";
|
||||
import {
|
||||
blockText,
|
||||
walk,
|
||||
@@ -2324,6 +2336,64 @@ export class DocmostClient {
|
||||
};
|
||||
}
|
||||
|
||||
/** Plain text of each TOP-LEVEL block of `doc`, for anchor-failure hints. */
|
||||
private topLevelBlockTexts(doc: any): string[] {
|
||||
const content = doc && Array.isArray(doc.content) ? doc.content : [];
|
||||
return content
|
||||
.map((b: any) => blockPlainText(b))
|
||||
.filter((t: string) => t.length > 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* True when per-block anchoring failed but the (normalized) selection DOES
|
||||
* appear in the blocks' joined plain text — i.e. it straddles a block
|
||||
* boundary. Blocks are joined with a newline (collapsed to one space by
|
||||
* normalizeForMatch) so a selection whose parts are separated by a paragraph
|
||||
* break still matches. Callers only reach here after single-block anchoring
|
||||
* (incl. the markdown-strip fallback) has already failed.
|
||||
*/
|
||||
private selectionSpansMultipleBlocks(
|
||||
blockTexts: string[],
|
||||
selection: string,
|
||||
): boolean {
|
||||
const normSel = normalizeForMatch(selection).norm.trim();
|
||||
if (normSel.length === 0) return false;
|
||||
const joined = normalizeForMatch(blockTexts.join("\n")).norm;
|
||||
return joined.indexOf(normSel) !== -1;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the actionable error for a create_comment anchor MISS, porting
|
||||
* edit_page_text's self-correction affordances: an explicit "spans multiple
|
||||
* blocks" message when the selection straddles a block boundary, otherwise a
|
||||
* "closest block text" hint quoting the block that holds the selection's
|
||||
* longest token. `live` switches the wording between the pre-check (reading the
|
||||
* persisted page) and the post-create live-anchor failure (which rolls back).
|
||||
*/
|
||||
private anchorNotFoundError(
|
||||
doc: any,
|
||||
selection: string,
|
||||
live: boolean,
|
||||
): Error {
|
||||
const blockTexts = this.topLevelBlockTexts(doc);
|
||||
const rolled = live ? " The comment was rolled back." : "";
|
||||
if (this.selectionSpansMultipleBlocks(blockTexts, selection)) {
|
||||
return new Error(
|
||||
"create_comment: the selection spans multiple blocks; anchor on a " +
|
||||
"contiguous fragment within a SINGLE paragraph/block (<=250 chars)." +
|
||||
rolled,
|
||||
);
|
||||
}
|
||||
const where = live ? "in the live document" : "in the page";
|
||||
return new Error(
|
||||
`create_comment: could not find the selection text ${where} to anchor ` +
|
||||
"the comment. Provide the EXACT contiguous text from a single " +
|
||||
"paragraph/block (<=250 chars)." +
|
||||
closestBlockHint(blockTexts, selection) +
|
||||
rolled,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create an inline comment anchored to its `selection` text, or a reply.
|
||||
*
|
||||
@@ -2385,6 +2455,10 @@ export class DocmostClient {
|
||||
// Captured in the pre-check below (which already reads the page) and used as
|
||||
// payload.selection. Ordinary comments keep sending the raw agent selection.
|
||||
let anchoredSelection: string | null = null;
|
||||
// Set when the anchor matched only after stripping markdown from the
|
||||
// selection (the strip fallback); surfaced as a soft warning like
|
||||
// edit_page_text does, so a stale-markdown selection is flagged.
|
||||
let anchorNormalized = false;
|
||||
|
||||
// For a top-level comment, fail BEFORE creating anything when the selection
|
||||
// is not present in the persisted document — this avoids leaving an orphan
|
||||
@@ -2400,10 +2474,7 @@ export class DocmostClient {
|
||||
// rejected BEFORE creating the comment.
|
||||
const matches = countAnchorMatches(page.content, selection);
|
||||
if (matches === 0) {
|
||||
throw new Error(
|
||||
"create_comment: could not find the selection text in the page to anchor the comment. " +
|
||||
"Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).",
|
||||
);
|
||||
throw this.anchorNotFoundError(page.content, selection, false);
|
||||
}
|
||||
if (matches >= 2) {
|
||||
throw new Error(
|
||||
@@ -2417,18 +2488,27 @@ export class DocmostClient {
|
||||
// null despite countAnchorMatches===1 (shouldn't happen), fall back to
|
||||
// the raw agent selection below rather than crash.
|
||||
anchoredSelection = getAnchoredText(page.content, selection);
|
||||
} else if (!canAnchorInDoc(page.content, selection)) {
|
||||
throw new Error(
|
||||
"create_comment: could not find the selection text in the page to anchor the comment. " +
|
||||
"Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).",
|
||||
);
|
||||
anchorNormalized = resolveAnchorSelection(
|
||||
page.content,
|
||||
selection,
|
||||
).normalized;
|
||||
} else {
|
||||
const resolved = resolveAnchorSelection(page.content, selection);
|
||||
if (!resolved.found) {
|
||||
throw this.anchorNotFoundError(page.content, selection, false);
|
||||
}
|
||||
anchorNormalized = resolved.normalized;
|
||||
}
|
||||
} catch (e) {
|
||||
// Rethrow our own "not found"/"ambiguous" errors; swallow read/network
|
||||
// errors so the live anchor step can still try (and enforce) anchoring.
|
||||
// Rethrow our own "not found"/"ambiguous"/"spans multiple blocks" errors;
|
||||
// swallow read/network errors so the live anchor step can still try (and
|
||||
// enforce) anchoring.
|
||||
if (
|
||||
e instanceof Error &&
|
||||
(e.message.startsWith("create_comment: could not find the selection") ||
|
||||
e.message.startsWith(
|
||||
"create_comment: the selection spans multiple blocks",
|
||||
) ||
|
||||
e.message.startsWith(
|
||||
"create_comment: the suggestion's selection is ambiguous",
|
||||
))
|
||||
@@ -2500,6 +2580,10 @@ export class DocmostClient {
|
||||
// Set inside the transform when a suggestion's live anchor is ambiguous
|
||||
// (>=2 occurrences), so the rollback path can surface the right error.
|
||||
let ambiguousInLiveDoc = false;
|
||||
// Captured inside the transform on a not-found abort, so the rollback path
|
||||
// can surface the closest-block / spans-multiple-blocks hint built from the
|
||||
// LIVE document (the pre-check page is not in scope there).
|
||||
let liveNotFoundError: Error | null = null;
|
||||
try {
|
||||
const collabToken = await this.getCollabTokenWithReauth();
|
||||
// Open the collab doc by the canonical UUID, never the slugId (#260). The
|
||||
@@ -2527,6 +2611,13 @@ export class DocmostClient {
|
||||
const liveCount = countAnchorMatches(doc, selection as string);
|
||||
if (liveCount !== 1) {
|
||||
ambiguousInLiveDoc = liveCount >= 2;
|
||||
if (liveCount === 0) {
|
||||
liveNotFoundError = this.anchorNotFoundError(
|
||||
doc,
|
||||
selection as string,
|
||||
true,
|
||||
);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
}
|
||||
@@ -2536,6 +2627,11 @@ export class DocmostClient {
|
||||
}
|
||||
// Selection text not found in the LIVE document: abort the write. The
|
||||
// rollback + throw below turns this into a hard error.
|
||||
liveNotFoundError = this.anchorNotFoundError(
|
||||
doc,
|
||||
selection as string,
|
||||
true,
|
||||
);
|
||||
return null;
|
||||
},
|
||||
);
|
||||
@@ -2552,13 +2648,28 @@ export class DocmostClient {
|
||||
// suggestion, was ambiguous) in the live document. Roll back the comment
|
||||
// and surface a hard error.
|
||||
await this.safeDeleteComment(newCommentId);
|
||||
throw new Error(
|
||||
ambiguousInLiveDoc
|
||||
? "create_comment: the suggestion's selection is ambiguous in the live document (multiple occurrences); the comment was rolled back. Expand the selection with surrounding context so it is unique."
|
||||
: "create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back",
|
||||
if (ambiguousInLiveDoc) {
|
||||
throw new Error(
|
||||
"create_comment: the suggestion's selection is ambiguous in the live document (multiple occurrences); the comment was rolled back. Expand the selection with surrounding context so it is unique.",
|
||||
);
|
||||
}
|
||||
throw (
|
||||
liveNotFoundError ??
|
||||
new Error(
|
||||
"create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back",
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
// Soft warning (like edit_page_text): the selection only matched after
|
||||
// stripping markdown, so the caller likely quoted a styled fragment.
|
||||
if (anchorNormalized) {
|
||||
result.warning =
|
||||
"The selection matched only after stripping markdown syntax; the comment " +
|
||||
"was anchored on the document's plain text. Copy the selection verbatim " +
|
||||
"from get_page / search_in_page output to avoid this.";
|
||||
}
|
||||
|
||||
result.anchored = true;
|
||||
return result;
|
||||
}
|
||||
@@ -3282,6 +3393,453 @@ export class DocmostClient {
|
||||
});
|
||||
}
|
||||
|
||||
// --- draw.io diagrams (issue #423) ---
|
||||
|
||||
/**
|
||||
* Upload a ready-made byte buffer as a page attachment via the same
|
||||
* multipart /files/upload endpoint uploadImage uses. Split out as its own
|
||||
* (overridable) seam so drawio_create/update can upload the generated
|
||||
* `.drawio.svg` without going through the URL-fetch path, and so tests can
|
||||
* stub the network. Mirrors uploadImage's fresh-FormData + one-shot 401/403
|
||||
* re-auth handling (a FormData body is single-use, so it must be rebuilt per
|
||||
* attempt).
|
||||
*/
|
||||
protected async uploadAttachmentBuffer(
|
||||
pageId: string,
|
||||
buffer: Buffer,
|
||||
fileName: string,
|
||||
mime: string,
|
||||
): Promise<{ id: string; fileName: string; fileSize: number }> {
|
||||
await this.ensureAuthenticated();
|
||||
const buildForm = () => {
|
||||
const form = new FormData();
|
||||
form.append("pageId", pageId);
|
||||
form.append("file", buffer, { filename: fileName, contentType: mime });
|
||||
return form;
|
||||
};
|
||||
const uploadUrl = `${this.apiUrl}/files/upload`;
|
||||
let response;
|
||||
try {
|
||||
const form = buildForm();
|
||||
response = await axios.post(uploadUrl, form, {
|
||||
headers: {
|
||||
...form.getHeaders(),
|
||||
Authorization: this.client.defaults.headers.common["Authorization"],
|
||||
},
|
||||
timeout: 60000,
|
||||
});
|
||||
} catch (error) {
|
||||
if (
|
||||
axios.isAxiosError(error) &&
|
||||
(error.response?.status === 401 || error.response?.status === 403)
|
||||
) {
|
||||
await this.login();
|
||||
const form2 = buildForm();
|
||||
response = await axios.post(uploadUrl, form2, {
|
||||
headers: {
|
||||
...form2.getHeaders(),
|
||||
Authorization: this.client.defaults.headers.common["Authorization"],
|
||||
},
|
||||
timeout: 60000,
|
||||
});
|
||||
} else if (axios.isAxiosError(error)) {
|
||||
if (process.env.DEBUG) {
|
||||
console.error(
|
||||
"Attachment upload failed; response body:",
|
||||
JSON.stringify(error.response?.data),
|
||||
);
|
||||
}
|
||||
throw new Error(
|
||||
`Attachment upload failed: ${error.response?.status} ${error.response?.statusText}`,
|
||||
);
|
||||
} else {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
const att = response.data?.data ?? response.data;
|
||||
if (!att?.id || !att?.fileName) {
|
||||
throw new Error(
|
||||
"Unexpected /files/upload response: " + JSON.stringify(response.data),
|
||||
);
|
||||
}
|
||||
return {
|
||||
id: att.id,
|
||||
fileName: att.fileName,
|
||||
fileSize: att.fileSize ?? buffer.length,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch a stored `.drawio.svg` attachment as text. Overridable seam over
|
||||
* fetchInternalFile (the authed loopback fetch, which also rejects any
|
||||
* traversal/SSRF src) so drawio_get/update can read the current diagram and
|
||||
* tests can stub the bytes.
|
||||
*/
|
||||
protected async fetchAttachmentText(src: string): Promise<string> {
|
||||
const { buffer } = await this.fetchInternalFile(src);
|
||||
return buffer.toString("utf-8");
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a drawio node on a page by `attrs.id` or `#<index>` and return the
|
||||
* node plus its ref. Throws a clear error if the ref does not resolve to a
|
||||
* drawio node.
|
||||
*/
|
||||
private async resolveDrawioNode(
|
||||
pageId: string,
|
||||
node: string,
|
||||
): Promise<{ node: any; ref: string }> {
|
||||
const data = await this.getPageRaw(pageId);
|
||||
const hit = getNodeByRef(
|
||||
data.content ?? { type: "doc", content: [] },
|
||||
node,
|
||||
);
|
||||
if (!hit) {
|
||||
throw new Error(
|
||||
`drawio: no node found for "${node}" on page ${pageId} (use the drawio node's attrs.id or "#<index>" from get_outline)`,
|
||||
);
|
||||
}
|
||||
if (hit.type !== "drawio") {
|
||||
throw new Error(
|
||||
`drawio: node "${node}" on page ${pageId} is a ${hit.type}, not a drawio diagram`,
|
||||
);
|
||||
}
|
||||
return { node: hit.node, ref: node };
|
||||
}
|
||||
|
||||
/**
|
||||
* Read a drawio diagram as mxGraph XML (default) or as the raw `.drawio.svg`.
|
||||
* Runs the decode chain (base64/entity content= → drawio file → nested XML or
|
||||
* pako-inflated compressed <diagram>). The returned `hash` is the
|
||||
* optimistic-lock key for drawio_update.
|
||||
*/
|
||||
async drawioGet(
|
||||
pageId: string,
|
||||
node: string,
|
||||
format: "xml" | "svg" = "xml",
|
||||
): Promise<{
|
||||
pageId: string;
|
||||
nodeId: string;
|
||||
format: "xml" | "svg";
|
||||
content: string;
|
||||
meta: {
|
||||
attachmentId: string | null;
|
||||
title: string | null;
|
||||
width: number | null;
|
||||
height: number | null;
|
||||
cellCount: number;
|
||||
hash: string;
|
||||
};
|
||||
}> {
|
||||
await this.ensureAuthenticated();
|
||||
const { node: drawio } = await this.resolveDrawioNode(pageId, node);
|
||||
const attrs = drawio.attrs || {};
|
||||
const src = attrs.src;
|
||||
if (!src) {
|
||||
throw new Error(
|
||||
`drawio: node "${node}" on page ${pageId} has no src to read`,
|
||||
);
|
||||
}
|
||||
const svg = await this.fetchAttachmentText(src);
|
||||
const modelXml = decodeDrawioSvg(svg);
|
||||
const meta = {
|
||||
attachmentId: attrs.attachmentId ?? null,
|
||||
title: attrs.title ?? null,
|
||||
width: attrs.width != null ? Number(attrs.width) : null,
|
||||
height: attrs.height != null ? Number(attrs.height) : null,
|
||||
cellCount: countUserCells(modelXml),
|
||||
hash: mxHash(modelXml),
|
||||
};
|
||||
return {
|
||||
pageId,
|
||||
nodeId: attrs.id ?? node,
|
||||
format,
|
||||
content: format === "svg" ? svg : normalizeXml(modelXml),
|
||||
meta,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a drawio diagram from mxGraph XML: lint → schematic SVG preview
|
||||
* (pure TS) → build the `.drawio.svg` (createDrawioSvg contract) → create the
|
||||
* attachment → insert a `drawio` node before/after an anchor or appended.
|
||||
* `xml` is a bare `<mxGraphModel>` or a list of `<mxCell>` (the server wraps
|
||||
* it and adds the id=0/id=1 sentinels).
|
||||
*/
|
||||
async drawioCreate(
|
||||
pageId: string,
|
||||
where: {
|
||||
position: "before" | "after" | "append";
|
||||
anchorNodeId?: string;
|
||||
anchorText?: string;
|
||||
},
|
||||
xml: string,
|
||||
title?: string,
|
||||
): Promise<{
|
||||
success: boolean;
|
||||
nodeId: string;
|
||||
attachmentId: string;
|
||||
warnings: string[];
|
||||
verify?: any;
|
||||
}> {
|
||||
await this.ensureAuthenticated();
|
||||
if (
|
||||
!where ||
|
||||
(where.position !== "before" &&
|
||||
where.position !== "after" &&
|
||||
where.position !== "append")
|
||||
) {
|
||||
throw new Error(
|
||||
'drawio_create: `where.position` must be one of "before", "after", "append"',
|
||||
);
|
||||
}
|
||||
if (where.position === "before" || where.position === "after") {
|
||||
const hasId =
|
||||
typeof where.anchorNodeId === "string" && where.anchorNodeId.length > 0;
|
||||
const hasText =
|
||||
typeof where.anchorText === "string" && where.anchorText.length > 0;
|
||||
if (hasId === hasText) {
|
||||
throw new Error(
|
||||
`drawio_create: position "${where.position}" requires exactly one of anchorNodeId or anchorText`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Pre-write pipeline (throws a structured DrawioLintError on any violation).
|
||||
const prepared = prepareModel(xml);
|
||||
const inner = renderDiagramShapes(prepared.cells, prepared.bbox);
|
||||
const diagramTitle = title || "Page-1";
|
||||
const svg = buildDrawioSvg(prepared.modelXml, inner, prepared.bbox, diagramTitle);
|
||||
|
||||
const att = await this.uploadAttachmentBuffer(
|
||||
pageId,
|
||||
Buffer.from(svg, "utf-8"),
|
||||
"diagram.drawio.svg",
|
||||
"image/svg+xml",
|
||||
);
|
||||
|
||||
// NOTE: no `id` attribute is set here. The vendored `drawio` node schema
|
||||
// (diagramAttributes) declares no `id`, so any block id would be silently
|
||||
// dropped by PMNode.fromJSON on save and the returned handle would fail to
|
||||
// resolve. The addressable handle is the node's "#<index>" (like image/table
|
||||
// nodes), computed after the insert below.
|
||||
const drawioNode: any = {
|
||||
type: "drawio",
|
||||
attrs: {
|
||||
src: `/api/files/${att.id}/${att.fileName}`,
|
||||
attachmentId: att.id,
|
||||
width: prepared.bbox.width,
|
||||
height: prepared.bbox.height,
|
||||
align: "center",
|
||||
},
|
||||
};
|
||||
if (title) drawioNode.attrs.title = title;
|
||||
// Reuse the existing URL trust boundary (rejects unsafe src schemes).
|
||||
this.validateDocUrls(drawioNode);
|
||||
|
||||
const collabToken = await this.getCollabTokenWithReauth();
|
||||
const pageUuid = await this.resolvePageId(pageId);
|
||||
|
||||
let inserted = false;
|
||||
let insertedIndex = -1;
|
||||
const mutation = await this.mutatePage(
|
||||
pageUuid,
|
||||
collabToken,
|
||||
this.apiUrl,
|
||||
(liveDoc) => {
|
||||
inserted = false;
|
||||
insertedIndex = -1;
|
||||
const { doc: nd, inserted: ins } = insertNodeRelative(
|
||||
liveDoc,
|
||||
drawioNode,
|
||||
where,
|
||||
);
|
||||
inserted = ins;
|
||||
if (!inserted) return null; // anchor not found -> skip the write
|
||||
// Locate the freshly-inserted node to derive its "#<index>" handle. The
|
||||
// just-uploaded attachmentId is unique, so it identifies our node.
|
||||
if (Array.isArray(nd.content)) {
|
||||
insertedIndex = nd.content.findIndex(
|
||||
(b: any) =>
|
||||
b &&
|
||||
b.type === "drawio" &&
|
||||
b.attrs &&
|
||||
b.attrs.attachmentId === att.id,
|
||||
);
|
||||
}
|
||||
return nd;
|
||||
},
|
||||
);
|
||||
|
||||
if (!inserted) {
|
||||
const anchorDesc = where.anchorNodeId
|
||||
? `anchorNodeId "${where.anchorNodeId}"`
|
||||
: `anchorText "${where.anchorText}"`;
|
||||
throw new Error(
|
||||
`drawio_create: anchor not found (${anchorDesc}) on page ${pageId}. The diagram attachment ${att.id} is now an unreferenced orphan.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (insertedIndex < 0) {
|
||||
// The node was inserted nested (e.g. inside a callout/table cell via an
|
||||
// anchor), where "#<index>" — which addresses only top-level blocks —
|
||||
// cannot reference it. drawio nodes carry no persisted id, so there is no
|
||||
// stable handle for a nested diagram.
|
||||
throw new Error(
|
||||
`drawio_create: the diagram was inserted on page ${pageId} but not as a ` +
|
||||
`top-level block, so it has no addressable "#<index>" handle. Anchor ` +
|
||||
`on a top-level block (or append) so the diagram can be re-read.`,
|
||||
);
|
||||
}
|
||||
|
||||
// The returned handle is POSITIONAL ("#<index>"): valid for the immediate
|
||||
// create -> get/update flow, but re-resolve via get_outline if the document
|
||||
// structure changes (blocks added/removed before it shift the index).
|
||||
const nodeId = `#${insertedIndex}`;
|
||||
|
||||
return {
|
||||
success: true,
|
||||
nodeId,
|
||||
attachmentId: att.id,
|
||||
warnings: prepared.warnings,
|
||||
verify: mutation.verify,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Full-replacement update of a drawio diagram. `baseHash` is MANDATORY: it is
|
||||
* compared against the hash of the diagram's CURRENT XML (from drawio_get);
|
||||
* any mismatch means a human or another agent edited the diagram after the
|
||||
* read, so the write is refused with a conflict error. On success the new
|
||||
* `.drawio.svg` is uploaded as a FRESH attachment (in-place byte overwrite is
|
||||
* avoided — some Docmost versions corrupt an attachment on overwrite, exactly
|
||||
* as replaceImage documents) and the node is repointed with new dimensions.
|
||||
*/
|
||||
async drawioUpdate(
|
||||
pageId: string,
|
||||
node: string,
|
||||
xml: string,
|
||||
baseHash: string,
|
||||
): Promise<{
|
||||
success: boolean;
|
||||
nodeId: string;
|
||||
attachmentId: string;
|
||||
warnings: string[];
|
||||
verify?: any;
|
||||
}> {
|
||||
await this.ensureAuthenticated();
|
||||
if (typeof baseHash !== "string" || baseHash.length === 0) {
|
||||
throw new Error(
|
||||
"drawio_update: baseHash is mandatory — read the diagram with drawio_get first and pass back its meta.hash",
|
||||
);
|
||||
}
|
||||
|
||||
// Resolve the node and read the CURRENT diagram to enforce the optimistic
|
||||
// lock before doing any write or upload.
|
||||
const { node: drawio, ref } = await this.resolveDrawioNode(pageId, node);
|
||||
const oldAttrs = drawio.attrs || {};
|
||||
const oldSrc = oldAttrs.src;
|
||||
// The returned handle is the caller-supplied reference. drawio nodes carry
|
||||
// no persisted id, so `ref` (an "#<index>" or a rare legacy attrs.id) is the
|
||||
// honest identifier to hand back.
|
||||
const nodeId = oldAttrs.id ?? ref;
|
||||
if (!oldSrc) {
|
||||
throw new Error(
|
||||
`drawio_update: node "${node}" on page ${pageId} has no src to compare against`,
|
||||
);
|
||||
}
|
||||
const currentSvg = await this.fetchAttachmentText(oldSrc);
|
||||
const currentHash = mxHash(decodeDrawioSvg(currentSvg));
|
||||
if (currentHash !== baseHash) {
|
||||
throw new Error(
|
||||
`drawio_update: conflict — the diagram changed since it was read ` +
|
||||
`(baseHash ${baseHash} != current ${currentHash}). Re-read it with drawio_get and retry.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Pipeline for the new content (throws a structured DrawioLintError).
|
||||
const prepared = prepareModel(xml);
|
||||
const inner = renderDiagramShapes(prepared.cells, prepared.bbox);
|
||||
const diagramTitle = oldAttrs.title || "Page-1";
|
||||
const svg = buildDrawioSvg(prepared.modelXml, inner, prepared.bbox, diagramTitle);
|
||||
|
||||
const att = await this.uploadAttachmentBuffer(
|
||||
pageId,
|
||||
Buffer.from(svg, "utf-8"),
|
||||
"diagram.drawio.svg",
|
||||
"image/svg+xml",
|
||||
);
|
||||
const newSrc = `/api/files/${att.id}/${att.fileName}`;
|
||||
|
||||
const collabToken = await this.getCollabTokenWithReauth();
|
||||
const pageUuid = await this.resolvePageId(pageId);
|
||||
|
||||
let repointed = 0;
|
||||
const repoint = (n: any) => {
|
||||
n.attrs = {
|
||||
...n.attrs,
|
||||
src: newSrc,
|
||||
attachmentId: att.id,
|
||||
width: prepared.bbox.width,
|
||||
height: prepared.bbox.height,
|
||||
};
|
||||
repointed++;
|
||||
};
|
||||
|
||||
const mutation = await this.mutatePage(
|
||||
pageUuid,
|
||||
collabToken,
|
||||
this.apiUrl,
|
||||
(liveDoc) => {
|
||||
repointed = 0;
|
||||
const doc =
|
||||
liveDoc && liveDoc.type === "doc"
|
||||
? liveDoc
|
||||
: { type: "doc", content: [] };
|
||||
if (!Array.isArray(doc.content)) doc.content = [];
|
||||
// Repoint ONLY the resolved node — never every node that happens to
|
||||
// share this attachmentId (a copied diagram is two nodes with one
|
||||
// attachmentId; keying on it would clobber both). Re-resolve the same
|
||||
// handle against the live doc and walk to its exact position.
|
||||
const hit = getNodeByRef(doc, ref);
|
||||
if (!hit || hit.type !== "drawio") return null; // vanished/changed -> skip
|
||||
let target: any = doc;
|
||||
for (const idx of hit.path) {
|
||||
if (!target || !Array.isArray(target.content)) {
|
||||
target = null;
|
||||
break;
|
||||
}
|
||||
target = target.content[idx];
|
||||
}
|
||||
if (!target || target.type !== "drawio") return null;
|
||||
repoint(target);
|
||||
if (repointed === 0) return null; // node vanished concurrently -> skip
|
||||
return doc;
|
||||
},
|
||||
);
|
||||
|
||||
if (repointed === 0) {
|
||||
return {
|
||||
success: true,
|
||||
nodeId,
|
||||
attachmentId: att.id,
|
||||
warnings: [
|
||||
...prepared.warnings,
|
||||
"target drawio node was removed concurrently; uploaded attachment is unreferenced",
|
||||
],
|
||||
verify: mutation.verify,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
success: true,
|
||||
nodeId,
|
||||
attachmentId: att.id,
|
||||
warnings: prepared.warnings,
|
||||
verify: mutation.verify,
|
||||
};
|
||||
}
|
||||
|
||||
// --- Page history / diff / transform ---
|
||||
|
||||
/**
|
||||
|
||||
@@ -52,7 +52,7 @@ const VERSION = packageJson.version;
|
||||
export const SERVER_INSTRUCTIONS =
|
||||
"Docmost editing guide — choose the tool by intent.\n" +
|
||||
"READ: find a page -> search (workspace-wide full-text); list -> list_pages / list_spaces. Locate blocks and their ids CHEAPLY -> get_outline (compact top-level map; start here, not get_page_json). One block's subtree -> get_node (by attrs.id, or \"#<index>\" for tables, which carry no id). Find every occurrence of a string/regex ON a page (and where each is) -> search_in_page, NOT block-by-block get_node — it returns each hit's node ref + block index + context for a targeted comment. Whole page -> get_page (Markdown, lossy; inline <span data-comment-id> tags are comment anchors — markup, not text) or get_page_json (lossless ProseMirror with block ids). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stash_page (returns a short-lived anonymous URL).\n" +
|
||||
"EDIT: fix wording/typos/numbers -> edit_page_text (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patch_node (by attrs.id from get_outline). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Tables -> table_get / table_update_cell / table_insert_row / table_delete_row (address by \"#<index>\" from get_outline; table nodes have no attrs.id). Images -> insert_image (add from a web URL) / replace_image (swap an existing image). Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" +
|
||||
"EDIT: fix wording/typos/numbers -> edit_page_text (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patch_node (by attrs.id from get_outline). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Tables -> table_get / table_update_cell / table_insert_row / table_delete_row (address by \"#<index>\" from get_outline; table nodes have no attrs.id). Images -> insert_image (add from a web URL) / replace_image (swap an existing image). Draw.io diagrams -> drawio_create (create from mxGraph XML and insert), drawio_get (read a diagram as mxGraph XML + a hash), drawio_update (replace a diagram; pass the hash from drawio_get as baseHash for optimistic locking). Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" +
|
||||
"PAGES: new -> create_page (Markdown). Rename (title only) -> rename_page. Move -> move_page. Delete -> delete_page (SOFT delete — the page goes to trash and is restorable; nothing is permanent). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Sharing -> share_page / unshare_page / list_shares; share_page makes the page PUBLICLY accessible — do it only when explicitly asked.\n" +
|
||||
"COMMENTS: create_comment is always inline and requires an EXACT selection — contiguous text from a single block, <=250 chars (fails rather than leaving an unanchored comment); reply to a thread via parentCommentId. Propose a concrete text fix for one-click human approval -> create_comment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Manage -> list_comments, update_comment, resolve_comment (resolve/reopen, reversible — prefer over delete to close), delete_comment, check_new_comments.\n" +
|
||||
"HISTORY: review what changed -> diff_page_versions (a historyId vs current, or two versions). List saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown.";
|
||||
@@ -465,6 +465,38 @@ registerShared(
|
||||
},
|
||||
);
|
||||
|
||||
// Tool: drawio_get — read a draw.io diagram as mxGraph XML (or the raw SVG).
|
||||
registerShared(
|
||||
SHARED_TOOL_SPECS.drawioGet,
|
||||
async ({ pageId, node, format }) => {
|
||||
const result = await docmostClient.drawioGet(pageId, node, format ?? "xml");
|
||||
return jsonContent(result);
|
||||
},
|
||||
);
|
||||
|
||||
// Tool: drawio_create — lint mxGraph XML, build the .drawio.svg, insert a node.
|
||||
registerShared(
|
||||
SHARED_TOOL_SPECS.drawioCreate,
|
||||
async ({ pageId, xml, position, anchorNodeId, anchorText, title }) => {
|
||||
const result = await docmostClient.drawioCreate(
|
||||
pageId,
|
||||
{ position, anchorNodeId, anchorText },
|
||||
xml,
|
||||
title,
|
||||
);
|
||||
return jsonContent(result);
|
||||
},
|
||||
);
|
||||
|
||||
// Tool: drawio_update — optimistic-locked full replacement of a diagram.
|
||||
registerShared(
|
||||
SHARED_TOOL_SPECS.drawioUpdate,
|
||||
async ({ pageId, node, xml, baseHash }) => {
|
||||
const result = await docmostClient.drawioUpdate(pageId, node, xml, baseHash);
|
||||
return jsonContent(result);
|
||||
},
|
||||
);
|
||||
|
||||
// Tool: share_page
|
||||
// Schema + description now live in the shared registry (#294). The execute body
|
||||
// keeps this transport's own `searchIndexing ?? true` default.
|
||||
|
||||
@@ -17,8 +17,23 @@
|
||||
* comparing and match across maximal runs of consecutive text nodes within a
|
||||
* single block, while mapping every normalized character back to its raw index
|
||||
* so the mark lands on the exact original characters.
|
||||
*
|
||||
* MARKDOWN-STRIP FALLBACK: when the agent copies a selection that still carries
|
||||
* inline markdown (`**bold**`, `` `code` ``, `[t](u)`), the raw locator will not
|
||||
* match the document's plain text. Exactly like edit_page_text's json-edit
|
||||
* fallback, we first try the verbatim selection and, ONLY if it anchors nowhere
|
||||
* in the whole document, retry with `stripInlineMarkdown` applied. `canAnchorInDoc`,
|
||||
* `getAnchoredText` and `applyAnchorInDoc` share this decision via
|
||||
* `resolveAnchorSelection`. `countAnchorMatches` keeps its OWN parallel exact-wins
|
||||
* implementation (it needs a raw match COUNT, not a single resolved locator), kept
|
||||
* deliberately in sync with `resolveAnchorSelection`: raw match ⇒ use raw, else fall
|
||||
* back to the stripped count. All four therefore agree on which locator matched —
|
||||
* the suggestion-uniqueness gate depends on count and can/get never disagreeing, so
|
||||
* these two exact-wins implementations MUST stay in sync if either is changed.
|
||||
*/
|
||||
|
||||
import { stripInlineMarkdown } from "./text-normalize.js";
|
||||
|
||||
/** Typographic double-quote variants mapped to ASCII `"`. */
|
||||
const DOUBLE_QUOTES = "«»„“”‟〝〞"";
|
||||
/** Typographic single-quote/apostrophe variants mapped to ASCII `'`. */
|
||||
@@ -214,15 +229,17 @@ function reconstructRawText(blockContent: any[], match: AnchorMatch): string {
|
||||
* un-appliable (spurious 409).
|
||||
*/
|
||||
export function getAnchoredText(doc: any, selection: string): string | null {
|
||||
const { selection: effective, found } = resolveAnchorSelection(doc, selection);
|
||||
if (!found) return null;
|
||||
const visit = (node: any, depth: number): string | null => {
|
||||
if (depth > MAX_DEPTH || !node || typeof node !== "object") return null;
|
||||
if (!Array.isArray(node.content)) return null;
|
||||
const match = findAnchorInBlock(node.content, selection);
|
||||
const match = findAnchorInBlock(node.content, effective);
|
||||
if (match) return reconstructRawText(node.content, match);
|
||||
for (const child of node.content) {
|
||||
if (child && typeof child === "object" && Array.isArray(child.content)) {
|
||||
const found = visit(child, depth + 1);
|
||||
if (found !== null) return found;
|
||||
const foundText = visit(child, depth + 1);
|
||||
if (foundText !== null) return foundText;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
@@ -231,12 +248,11 @@ export function getAnchoredText(doc: any, selection: string): string | null {
|
||||
}
|
||||
|
||||
/**
|
||||
* Depth-first, document-order check for whether `selection` can be anchored
|
||||
* anywhere in `doc`. At each node with an array `content`, first try to match
|
||||
* within that node's own content, then recurse into children that themselves
|
||||
* have a `content` array.
|
||||
* RAW (no markdown-strip fallback) depth-first check that `selection` anchors
|
||||
* somewhere in `doc`. This is the primitive `resolveAnchorSelection` builds on;
|
||||
* public callers should use `canAnchorInDoc`, which adds the strip fallback.
|
||||
*/
|
||||
export function canAnchorInDoc(doc: any, selection: string): boolean {
|
||||
function rawCanAnchorInDoc(doc: any, selection: string): boolean {
|
||||
const visit = (node: any, depth: number): boolean => {
|
||||
if (depth > MAX_DEPTH || !node || typeof node !== "object") return false;
|
||||
if (!Array.isArray(node.content)) return false;
|
||||
@@ -251,6 +267,43 @@ export function canAnchorInDoc(doc: any, selection: string): boolean {
|
||||
return visit(doc, 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide the locator that ACTUALLY anchors `selection` in `doc`, applying the
|
||||
* markdown-strip fallback once (so every public entry point agrees):
|
||||
* - EXACT WINS: if the verbatim selection anchors anywhere, use it as-is.
|
||||
* - FALLBACK: only if the verbatim selection anchors nowhere, and the
|
||||
* markdown-stripped form differs and DOES anchor, use the stripped form and
|
||||
* flag `normalized` so callers can surface a soft warning.
|
||||
* - otherwise `found` is false and `selection` is returned unchanged.
|
||||
*
|
||||
* The stripped form is used ONLY to LOCATE the anchor; getAnchoredText still
|
||||
* reconstructs and stores the RAW document substring, so the strip never leaks
|
||||
* into what gets persisted.
|
||||
*/
|
||||
export function resolveAnchorSelection(
|
||||
doc: any,
|
||||
selection: string,
|
||||
): { selection: string; found: boolean; normalized: boolean } {
|
||||
if (rawCanAnchorInDoc(doc, selection)) {
|
||||
return { selection, found: true, normalized: false };
|
||||
}
|
||||
const stripped = stripInlineMarkdown(selection);
|
||||
if (stripped !== selection && rawCanAnchorInDoc(doc, stripped)) {
|
||||
return { selection: stripped, found: true, normalized: true };
|
||||
}
|
||||
return { selection, found: false, normalized: false };
|
||||
}
|
||||
|
||||
/**
|
||||
* Depth-first, document-order check for whether `selection` can be anchored
|
||||
* anywhere in `doc` (with the markdown-strip fallback). At each node with an
|
||||
* array `content`, first try to match within that node's own content, then
|
||||
* recurse into children that themselves have a `content` array.
|
||||
*/
|
||||
export function canAnchorInDoc(doc: any, selection: string): boolean {
|
||||
return resolveAnchorSelection(doc, selection).found;
|
||||
}
|
||||
|
||||
/**
|
||||
* Split the matched text nodes and splice the comment mark across the range.
|
||||
* `blockContent` is mutated IN PLACE. `match.startChild..endChild` are all text
|
||||
@@ -315,7 +368,7 @@ function spliceCommentMark(
|
||||
* not use this. (Note: counts OCCURRENCES, not just matching blocks, so two
|
||||
* occurrences inside one block are correctly reported as 2.)
|
||||
*/
|
||||
export function countAnchorMatches(doc: any, selection: string): number {
|
||||
function rawCountAnchorMatches(doc: any, selection: string): number {
|
||||
const normSel = normalizeForMatch(selection).norm.trim();
|
||||
if (normSel.length === 0) return 0;
|
||||
|
||||
@@ -369,6 +422,25 @@ export function countAnchorMatches(doc: any, selection: string): number {
|
||||
return total;
|
||||
}
|
||||
|
||||
/**
|
||||
* Uniqueness gate for suggestions, with the SAME markdown-strip fallback as the
|
||||
* other entry points so count never disagrees with can/get/apply. EXACT WINS: if
|
||||
* the verbatim selection occurs at all, return its raw occurrence count (so a
|
||||
* selection that is unique raw stays unique — the fallback never runs and cannot
|
||||
* introduce a spurious second match). Only when the verbatim selection is absent
|
||||
* do we count occurrences of the markdown-stripped form.
|
||||
*/
|
||||
export function countAnchorMatches(doc: any, selection: string): number {
|
||||
const raw = rawCountAnchorMatches(doc, selection);
|
||||
if (raw > 0) return raw;
|
||||
const stripped = stripInlineMarkdown(selection);
|
||||
if (stripped !== selection) {
|
||||
const strippedCount = rawCountAnchorMatches(doc, stripped);
|
||||
if (strippedCount > 0) return strippedCount;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Depth-first (same order as canAnchorInDoc) over `doc`; on the FIRST block
|
||||
* whose content matches `selection`, splice the comment mark across the matched
|
||||
@@ -380,10 +452,12 @@ export function applyAnchorInDoc(
|
||||
selection: string,
|
||||
commentId: string,
|
||||
): boolean {
|
||||
const { selection: effective, found } = resolveAnchorSelection(doc, selection);
|
||||
if (!found) return false;
|
||||
const visit = (node: any, depth: number): boolean => {
|
||||
if (depth > MAX_DEPTH || !node || typeof node !== "object") return false;
|
||||
if (!Array.isArray(node.content)) return false;
|
||||
const match = findAnchorInBlock(node.content, selection);
|
||||
const match = findAnchorInBlock(node.content, effective);
|
||||
if (match) {
|
||||
spliceCommentMark(node.content, match, commentId);
|
||||
return true;
|
||||
|
||||
@@ -0,0 +1,193 @@
|
||||
// Pure-TS schematic SVG preview for draw.io diagrams (issue #423, stage 1).
|
||||
//
|
||||
// HARD CONSTRAINT: no backend rendering. This is a dependency-free string
|
||||
// builder — given the parsed mxGraph cells it draws a rough schematic (rects,
|
||||
// ellipses, diamonds, edges + labels) that stands in as the diagram's visible
|
||||
// image UNTIL a human first opens it in the draw.io editor and saves, at which
|
||||
// point the client replaces this with the pixel-perfect export SVG. It is
|
||||
// deliberately approximate: it exists so a freshly-agent-created diagram is not
|
||||
// an empty box in the page.
|
||||
|
||||
import type { DrawioCell, DrawioBBox } from "./drawio-xml.js";
|
||||
import { absolutePos } from "./drawio-xml.js";
|
||||
|
||||
function esc(s: string): string {
|
||||
return s
|
||||
.replace(/&/g, "&")
|
||||
.replace(/</g, "<")
|
||||
.replace(/>/g, ">")
|
||||
.replace(/"/g, """);
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip HTML markup from a cell value (draw.io labels are HTML when html=1),
|
||||
* decode the handful of entities we care about, and collapse whitespace so the
|
||||
* label fits on the schematic. `<br>` becomes a space (this is a one-line
|
||||
* preview label, not a faithful multi-line render).
|
||||
*/
|
||||
function labelText(value: string): string {
|
||||
return value
|
||||
.replace(/<br\s*\/?>/gi, " ")
|
||||
.replace(/<[^>]+>/g, "")
|
||||
.replace(/</g, "<")
|
||||
.replace(/>/g, ">")
|
||||
.replace(/"/g, '"')
|
||||
.replace(/'|'/g, "'")
|
||||
.replace(/
| /gi, " ")
|
||||
.replace(/&/g, "&")
|
||||
.replace(/\s+/g, " ")
|
||||
.trim();
|
||||
}
|
||||
|
||||
function centeredLabel(cx: number, cy: number, value: string, color = "#000000"): string {
|
||||
const text = labelText(value);
|
||||
if (!text) return "";
|
||||
return (
|
||||
`<text x="${round(cx)}" y="${round(cy)}" ` +
|
||||
`font-family="Helvetica, Arial, sans-serif" font-size="12" ` +
|
||||
`text-anchor="middle" dominant-baseline="middle" fill="${esc(color)}">` +
|
||||
`${esc(text)}</text>`
|
||||
);
|
||||
}
|
||||
|
||||
function round(n: number): number {
|
||||
return Math.round(n * 100) / 100;
|
||||
}
|
||||
|
||||
interface ShapeKind {
|
||||
kind: "ellipse" | "rhombus" | "triangle" | "rect";
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide which schematic primitive to draw for a vertex. A shape can be named
|
||||
* either as the style's base token (e.g. "ellipse;…") or as a key (e.g.
|
||||
* "shape=rhombus" / "ellipse=1"), so both the base style and the map are
|
||||
* checked.
|
||||
*/
|
||||
function shapeKind(
|
||||
styleMap: Record<string, string>,
|
||||
baseStyle?: string,
|
||||
): ShapeKind {
|
||||
const shape = styleMap.shape ?? baseStyle;
|
||||
const has = (name: string) => shape === name || styleMap[name] != null;
|
||||
if (has("ellipse")) return { kind: "ellipse" };
|
||||
if (has("rhombus")) return { kind: "rhombus" };
|
||||
if (has("triangle")) return { kind: "triangle" };
|
||||
// Everything else — including unknown stencils (shape=mxgraph.*), swimlanes,
|
||||
// and plain boxes — is drawn as a (rounded) rectangle.
|
||||
return { kind: "rect" };
|
||||
}
|
||||
|
||||
function fill(styleMap: Record<string, string>): string {
|
||||
const c = styleMap.fillColor;
|
||||
if (!c || c.toLowerCase() === "none") return "#ffffff";
|
||||
return c;
|
||||
}
|
||||
|
||||
function stroke(styleMap: Record<string, string>): string {
|
||||
const c = styleMap.strokeColor;
|
||||
if (!c || c.toLowerCase() === "none") return "#000000";
|
||||
return c;
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the schematic shapes as the INNER content of the `.drawio.svg` (the
|
||||
* outer <svg> wrapper is added by drawio-xml.buildDrawioSvg). Coordinates are
|
||||
* absolute (container children are resolved via the parent chain).
|
||||
*/
|
||||
export function renderDiagramShapes(cells: DrawioCell[], _bbox: DrawioBBox): string {
|
||||
const byId = new Map(cells.map((c) => [c.id, c]));
|
||||
const parts: string[] = [];
|
||||
|
||||
// Edges first so vertices sit on top of their connectors.
|
||||
for (const c of cells) {
|
||||
if (!c.edge) continue;
|
||||
parts.push(renderEdge(c, byId));
|
||||
}
|
||||
|
||||
for (const c of cells) {
|
||||
if (!c.vertex || !c.geometry.hasGeometry) continue;
|
||||
const g = c.geometry;
|
||||
if (g.width == null || g.height == null) continue;
|
||||
const { x, y } = absolutePos(c, byId);
|
||||
parts.push(renderVertex(c, x, y, g.width, g.height));
|
||||
}
|
||||
|
||||
return `<g>${parts.filter(Boolean).join("")}</g>`;
|
||||
}
|
||||
|
||||
function renderVertex(
|
||||
c: DrawioCell,
|
||||
x: number,
|
||||
y: number,
|
||||
w: number,
|
||||
h: number,
|
||||
): string {
|
||||
const f = esc(fill(c.styleMap));
|
||||
const s = esc(stroke(c.styleMap));
|
||||
const { kind } = shapeKind(c.styleMap, c.baseStyle);
|
||||
const cx = x + w / 2;
|
||||
const cy = y + h / 2;
|
||||
let shape = "";
|
||||
switch (kind) {
|
||||
case "ellipse":
|
||||
shape =
|
||||
`<ellipse cx="${round(cx)}" cy="${round(cy)}" rx="${round(w / 2)}" ` +
|
||||
`ry="${round(h / 2)}" fill="${f}" stroke="${s}"/>`;
|
||||
break;
|
||||
case "rhombus": {
|
||||
const pts = [
|
||||
`${round(cx)},${round(y)}`,
|
||||
`${round(x + w)},${round(cy)}`,
|
||||
`${round(cx)},${round(y + h)}`,
|
||||
`${round(x)},${round(cy)}`,
|
||||
].join(" ");
|
||||
shape = `<polygon points="${pts}" fill="${f}" stroke="${s}"/>`;
|
||||
break;
|
||||
}
|
||||
case "triangle": {
|
||||
const pts = [
|
||||
`${round(x)},${round(y)}`,
|
||||
`${round(x + w)},${round(cy)}`,
|
||||
`${round(x)},${round(y + h)}`,
|
||||
].join(" ");
|
||||
shape = `<polygon points="${pts}" fill="${f}" stroke="${s}"/>`;
|
||||
break;
|
||||
}
|
||||
default: {
|
||||
const rounded = c.styleMap.rounded === "1";
|
||||
const rx = rounded ? Math.min(12, w / 2, h / 2) : 0;
|
||||
shape =
|
||||
`<rect x="${round(x)}" y="${round(y)}" width="${round(w)}" ` +
|
||||
`height="${round(h)}" rx="${round(rx)}" ry="${round(rx)}" ` +
|
||||
`fill="${f}" stroke="${s}"/>`;
|
||||
}
|
||||
}
|
||||
return shape + centeredLabel(cx, cy, c.value, c.styleMap.fontColor || "#000000");
|
||||
}
|
||||
|
||||
function renderEdge(c: DrawioCell, byId: Map<string, DrawioCell>): string {
|
||||
const src = c.source != null ? byId.get(c.source) : undefined;
|
||||
const tgt = c.target != null ? byId.get(c.target) : undefined;
|
||||
const p1 = anchorPoint(src, byId);
|
||||
const p2 = anchorPoint(tgt, byId);
|
||||
if (!p1 || !p2) return ""; // a floating endpoint with no fixed point: skip
|
||||
const line =
|
||||
`<line x1="${round(p1.x)}" y1="${round(p1.y)}" ` +
|
||||
`x2="${round(p2.x)}" y2="${round(p2.y)}" ` +
|
||||
`stroke="#000000" stroke-width="1"/>`;
|
||||
const mid = { x: (p1.x + p2.x) / 2, y: (p1.y + p2.y) / 2 };
|
||||
return line + centeredLabel(mid.x, mid.y, c.value);
|
||||
}
|
||||
|
||||
/** Center point of a vertex used as an edge anchor (approximate). */
|
||||
function anchorPoint(
|
||||
cell: DrawioCell | undefined,
|
||||
byId: Map<string, DrawioCell>,
|
||||
): { x: number; y: number } | null {
|
||||
if (!cell || !cell.geometry.hasGeometry) return null;
|
||||
const g = cell.geometry;
|
||||
if (g.width == null || g.height == null) return null;
|
||||
const { x, y } = absolutePos(cell, byId);
|
||||
return { x: x + g.width / 2, y: y + g.height / 2 };
|
||||
}
|
||||
@@ -0,0 +1,771 @@
|
||||
// draw.io (mxGraph) XML support for the MCP drawio tools (issue #423, stage 1).
|
||||
//
|
||||
// This module owns everything that is pure data-plumbing for draw.io diagrams:
|
||||
// - the DECODE CHAIN that turns a stored `diagram.drawio.svg` attachment back
|
||||
// into mxGraph XML (handles both the plain nested-XML form Docmost writes
|
||||
// and draw.io's own COMPRESSED `<diagram>` payload — base64 + raw-deflate);
|
||||
// - the ENCODE side that wraps mxGraph XML into the `.drawio.svg` attachment
|
||||
// using the exact same contract as the import service's createDrawioSvg;
|
||||
// - a deterministic LINTER that rejects the structural mistakes generators
|
||||
// make before anything is written (each violation carries the offending
|
||||
// cellId + position so the model can auto-retry);
|
||||
// - a stable HASH over the normalized XML, used as the optimistic-lock key.
|
||||
//
|
||||
// HARD CONSTRAINT: no backend rendering. Nothing here shells out or renders a
|
||||
// bitmap; the only runtime dependencies are jsdom (already used across this
|
||||
// package for XML parsing) and pako (raw-inflate for the compressed format).
|
||||
|
||||
import { createHash } from "node:crypto";
|
||||
import { JSDOM } from "jsdom";
|
||||
import pako from "pako";
|
||||
|
||||
// --- shared XML parser -----------------------------------------------------
|
||||
|
||||
// A single reusable JSDOM window; constructing one per parse is wasteful and
|
||||
// these tools are low-frequency. Only the DOMParser is used.
|
||||
let _window: any = null;
|
||||
function xmlWindow(): any {
|
||||
if (!_window) _window = new JSDOM("").window;
|
||||
return _window;
|
||||
}
|
||||
|
||||
/** Default mxGraphModel attributes used when the server wraps a cell list. */
|
||||
const DEFAULT_MODEL_ATTRS =
|
||||
'dx="0" dy="0" grid="1" gridSize="10" page="1" pageWidth="850" pageHeight="1100"';
|
||||
|
||||
// --- structured lint errors ------------------------------------------------
|
||||
|
||||
export interface DrawioLintIssue {
|
||||
/** Machine-readable rule id, e.g. "edge-geometry". */
|
||||
rule: string;
|
||||
/** Human-readable explanation the model can act on. */
|
||||
message: string;
|
||||
/** The offending cell's id, when the rule is cell-scoped. */
|
||||
cellId?: string;
|
||||
/** Extra location info: cell index in <root>, or a parser line:col. */
|
||||
position?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Thrown by the linter and by decode/prepare when the input is unusable. Carries
|
||||
* the full list of issues so the caller can surface a structured tool-error the
|
||||
* model auto-retries against.
|
||||
*/
|
||||
export class DrawioLintError extends Error {
|
||||
issues: DrawioLintIssue[];
|
||||
constructor(issues: DrawioLintIssue[]) {
|
||||
const summary = issues
|
||||
.map((i) => {
|
||||
const where = [
|
||||
i.cellId != null ? `cellId=${i.cellId}` : null,
|
||||
i.position != null ? `at ${i.position}` : null,
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join(", ");
|
||||
return `[${i.rule}] ${i.message}${where ? ` (${where})` : ""}`;
|
||||
})
|
||||
.join("; ");
|
||||
super(`drawio lint failed: ${summary}`);
|
||||
this.name = "DrawioLintError";
|
||||
this.issues = issues;
|
||||
}
|
||||
}
|
||||
|
||||
// --- parsed-cell model -----------------------------------------------------
|
||||
|
||||
export interface DrawioGeometry {
|
||||
x?: number;
|
||||
y?: number;
|
||||
width?: number;
|
||||
height?: number;
|
||||
relative: boolean;
|
||||
hasGeometry: boolean;
|
||||
}
|
||||
|
||||
export interface DrawioCell {
|
||||
id: string;
|
||||
parent?: string;
|
||||
source?: string;
|
||||
target?: string;
|
||||
vertex: boolean;
|
||||
edge: boolean;
|
||||
value: string;
|
||||
style: string;
|
||||
styleMap: Record<string, string>;
|
||||
/** Non-key/value leading token of the style (a base stylename), if any. */
|
||||
baseStyle?: string;
|
||||
geometry: DrawioGeometry;
|
||||
}
|
||||
|
||||
export interface DrawioBBox {
|
||||
width: number;
|
||||
height: number;
|
||||
}
|
||||
|
||||
// --- style parsing ---------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Parse a draw.io style string into { baseStyle, map }. Grammar:
|
||||
* [stylename;]key=value;key=value;...
|
||||
* A single leading token without '=' is the base stylename (e.g. "text" or
|
||||
* "ellipse"). Every other non-empty segment must be exactly one key=value pair.
|
||||
* Returns `null` (the segment index) on the first malformed segment so the
|
||||
* linter can report a precise error.
|
||||
*/
|
||||
export function parseStyle(
|
||||
style: string,
|
||||
): { baseStyle?: string; map: Record<string, string>; badSegment?: string } {
|
||||
const map: Record<string, string> = {};
|
||||
let baseStyle: string | undefined;
|
||||
const segments = style.split(";");
|
||||
for (let i = 0; i < segments.length; i++) {
|
||||
const seg = segments[i].trim();
|
||||
if (seg === "") continue; // trailing/empty segments are fine
|
||||
const eq = seg.indexOf("=");
|
||||
if (eq === -1) {
|
||||
// A bare token is only valid as the FIRST meaningful segment (base style).
|
||||
if (baseStyle === undefined && Object.keys(map).length === 0) {
|
||||
baseStyle = seg;
|
||||
continue;
|
||||
}
|
||||
return { baseStyle, map, badSegment: seg };
|
||||
}
|
||||
// A second '=' inside the same segment is malformed.
|
||||
if (seg.indexOf("=", eq + 1) !== -1) {
|
||||
return { baseStyle, map, badSegment: seg };
|
||||
}
|
||||
const key = seg.slice(0, eq).trim();
|
||||
const val = seg.slice(eq + 1).trim();
|
||||
if (key === "") return { baseStyle, map, badSegment: seg };
|
||||
map[key] = val;
|
||||
}
|
||||
return { baseStyle, map };
|
||||
}
|
||||
|
||||
// --- low-level XML helpers -------------------------------------------------
|
||||
|
||||
function parseXml(xml: string): { doc: any; error: string | null } {
|
||||
const parser = new (xmlWindow().DOMParser)();
|
||||
const doc = parser.parseFromString(xml, "application/xml");
|
||||
const err = doc.getElementsByTagName("parsererror");
|
||||
if (err.length > 0) {
|
||||
// jsdom prefixes the message with "line:col:" — keep it as the position.
|
||||
return { doc, error: (err[0].textContent || "malformed XML").trim() };
|
||||
}
|
||||
return { doc, error: null };
|
||||
}
|
||||
|
||||
function num(v: string | null): number | undefined {
|
||||
if (v == null || v === "") return undefined;
|
||||
const n = Number(v);
|
||||
return Number.isFinite(n) ? n : undefined;
|
||||
}
|
||||
|
||||
/** Extract the raw `<mxGraphModel …>…</mxGraphModel>` substring, or null. */
|
||||
function sliceModel(xml: string): string | null {
|
||||
const open = xml.indexOf("<mxGraphModel");
|
||||
if (open === -1) return null;
|
||||
const close = xml.indexOf("</mxGraphModel>", open);
|
||||
if (close === -1) {
|
||||
// Self-closed empty model, e.g. `<mxGraphModel .../>`.
|
||||
const selfClose = xml.indexOf("/>", open);
|
||||
if (selfClose !== -1) return xml.slice(open, selfClose + 2);
|
||||
return null;
|
||||
}
|
||||
return xml.slice(open, close + "</mxGraphModel>".length);
|
||||
}
|
||||
|
||||
// --- decode chain ----------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Read the `content=` attribute out of a `.drawio.svg` string. Docmost stores a
|
||||
* base64 payload there (createDrawioSvg); draw.io's own SVG export may store the
|
||||
* XML entity-encoded instead. The DOM decodes entities for us, so the caller
|
||||
* only has to distinguish "starts with '<'" (raw XML) from base64.
|
||||
*/
|
||||
export function extractContentAttr(svg: string): string {
|
||||
const { doc, error } = parseXml(svg);
|
||||
if (!error) {
|
||||
const root = doc.documentElement;
|
||||
if (root && root.hasAttribute && root.hasAttribute("content")) {
|
||||
return root.getAttribute("content") || "";
|
||||
}
|
||||
}
|
||||
// Fallback for a malformed wrapper: pull the attribute directly. The content
|
||||
// value itself never contains a double-quote (base64 / entity-encoded XML).
|
||||
const m = /content="([^"]*)"/.exec(svg);
|
||||
if (m) {
|
||||
// Decode the handful of XML entities a raw regex would leave encoded.
|
||||
return m[1]
|
||||
.replace(/</g, "<")
|
||||
.replace(/>/g, ">")
|
||||
.replace(/"/g, '"')
|
||||
.replace(/'/g, "'")
|
||||
.replace(/&/g, "&");
|
||||
}
|
||||
throw new Error("drawio: SVG has no content= attribute to decode");
|
||||
}
|
||||
|
||||
/**
|
||||
* Turn a decoded draw.io file (`<mxfile>` or a bare `<mxGraphModel>`, possibly
|
||||
* with a COMPRESSED `<diagram>` payload) into the mxGraphModel XML. For the
|
||||
* plain form the raw substring is returned verbatim so a round-trip stays
|
||||
* byte-stable; the compressed form is inflated (base64 → raw-deflate →
|
||||
* decodeURIComponent), which is how draw.io stores diagrams by default.
|
||||
*/
|
||||
export function decodeDrawioFileToModel(fileXml: string): string {
|
||||
// Plain, nested XML: return the model substring untouched (byte-stable).
|
||||
const sliced = sliceModel(fileXml);
|
||||
if (sliced) return sliced;
|
||||
|
||||
// Otherwise it must be the compressed `<diagram>…</diagram>` text payload.
|
||||
const open = fileXml.indexOf("<diagram");
|
||||
if (open !== -1) {
|
||||
const gt = fileXml.indexOf(">", open);
|
||||
const close = fileXml.indexOf("</diagram>", gt);
|
||||
if (gt !== -1 && close !== -1) {
|
||||
const payload = fileXml.slice(gt + 1, close).trim();
|
||||
if (payload) {
|
||||
const inflated = inflateDiagramPayload(payload);
|
||||
const model = sliceModel(inflated);
|
||||
if (model) return model;
|
||||
return inflated;
|
||||
}
|
||||
}
|
||||
}
|
||||
throw new Error(
|
||||
"drawio: could not decode file — no <mxGraphModel> and no compressed <diagram> payload",
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Upper bound on the inflated size of a compressed `<diagram>` payload
|
||||
* (decompression-bomb guard). `fetchInternalFile` caps the DOWNLOAD at 64 MiB,
|
||||
* but a tiny crafted compressed payload can inflate to gigabytes and OOM the
|
||||
* process. A real diagram's mxGraphModel XML is small (KBs to low MBs even for
|
||||
* large diagrams), so 16 MiB is far above any legitimate payload while keeping
|
||||
* memory bounded. Chars ~= bytes for the (mostly ASCII) URI-encoded XML.
|
||||
*/
|
||||
export const MAX_INFLATED_DIAGRAM_BYTES = 16 * 1024 * 1024;
|
||||
|
||||
/**
|
||||
* Inflate draw.io's compressed diagram payload:
|
||||
* base64-decode → raw-inflate (raw deflate, windowBits -15) →
|
||||
* decodeURIComponent.
|
||||
*
|
||||
* Uses pako's streaming Inflate so we can abort as soon as the decompressed
|
||||
* output exceeds MAX_INFLATED_DIAGRAM_BYTES — the full bomb is never
|
||||
* materialised in memory.
|
||||
*/
|
||||
export function inflateDiagramPayload(base64: string): string {
|
||||
const bytes = Buffer.from(base64, "base64");
|
||||
const inflator = new pako.Inflate({ raw: true, to: "string" });
|
||||
let total = 0;
|
||||
const passthrough = inflator.onData.bind(inflator);
|
||||
inflator.onData = (chunk: string | Uint8Array) => {
|
||||
total += chunk.length;
|
||||
if (total > MAX_INFLATED_DIAGRAM_BYTES) {
|
||||
// Throwing here propagates out of push(), aborting inflation immediately.
|
||||
throw new Error(
|
||||
`drawio: refusing to decode diagram — decompressed size exceeds ` +
|
||||
`${MAX_INFLATED_DIAGRAM_BYTES} bytes (possible decompression bomb)`,
|
||||
);
|
||||
}
|
||||
passthrough(chunk);
|
||||
};
|
||||
inflator.push(bytes, true);
|
||||
if (inflator.err) {
|
||||
throw new Error(
|
||||
`drawio: failed to inflate compressed <diagram> payload (${inflator.msg || inflator.err})`,
|
||||
);
|
||||
}
|
||||
const uriEncoded = inflator.result as string;
|
||||
return decodeURIComponent(uriEncoded);
|
||||
}
|
||||
|
||||
/** Full decode chain: `.drawio.svg` string → mxGraphModel XML. */
|
||||
export function decodeDrawioSvg(svg: string): string {
|
||||
const content = extractContentAttr(svg).trim();
|
||||
const fileXml = content.startsWith("<")
|
||||
? content
|
||||
: Buffer.from(content, "base64").toString("utf-8");
|
||||
return decodeDrawioFileToModel(fileXml);
|
||||
}
|
||||
|
||||
// --- encode side -----------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Wrap an mxGraphModel in the plain (uncompressed) `<mxfile><diagram>` envelope.
|
||||
* draw.io opens uncompressed XML fine, and staying uncompressed keeps the
|
||||
* write path deterministic and the round-trip byte-stable.
|
||||
*/
|
||||
export function encodeDrawioFile(modelXml: string, title = "Page-1"): string {
|
||||
const safeTitle = xmlEscape(title);
|
||||
return `<mxfile host="drawio"><diagram id="page-1" name="${safeTitle}">${modelXml}</diagram></mxfile>`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the `diagram.drawio.svg` attachment. Mirrors the import service's
|
||||
* createDrawioSvg contract exactly:
|
||||
* <svg xmlns=… xmlns:xlink=… content="${base64(drawioFile)}">${inner}</svg>
|
||||
* plus width/height/viewBox from the diagram bounding box and the schematic
|
||||
* preview as the visible children (`inner`).
|
||||
*/
|
||||
export function buildDrawioSvg(
|
||||
modelXml: string,
|
||||
inner: string,
|
||||
bbox: DrawioBBox,
|
||||
title = "Page-1",
|
||||
): string {
|
||||
const file = encodeDrawioFile(modelXml, title);
|
||||
const base64 = Buffer.from(file, "utf-8").toString("base64");
|
||||
const w = Math.max(1, Math.round(bbox.width));
|
||||
const h = Math.max(1, Math.round(bbox.height));
|
||||
return (
|
||||
`<svg xmlns="http://www.w3.org/2000/svg" ` +
|
||||
`xmlns:xlink="http://www.w3.org/1999/xlink" ` +
|
||||
`width="${w}" height="${h}" viewBox="0 0 ${w} ${h}" ` +
|
||||
`content="${base64}">${inner}</svg>`
|
||||
);
|
||||
}
|
||||
|
||||
function xmlEscape(s: string): string {
|
||||
return s
|
||||
.replace(/&/g, "&")
|
||||
.replace(/</g, "<")
|
||||
.replace(/>/g, ">")
|
||||
.replace(/"/g, """);
|
||||
}
|
||||
|
||||
// --- normalization + hash --------------------------------------------------
|
||||
|
||||
/**
|
||||
* Normalize mxGraph XML for hashing / stable comparison: drop the whitespace
|
||||
* between tags and trim. This is intentionally conservative — it never reorders
|
||||
* attributes or cells (that would be lossy) — so two documents hash equal iff
|
||||
* they differ only in inter-tag formatting.
|
||||
*/
|
||||
export function normalizeXml(xml: string): string {
|
||||
return xml.replace(/>\s+</g, "><").trim();
|
||||
}
|
||||
|
||||
/** Stable optimistic-lock hash over the normalized model XML (sha256, hex). */
|
||||
export function mxHash(modelXml: string): string {
|
||||
return createHash("sha256").update(normalizeXml(modelXml), "utf-8").digest("hex");
|
||||
}
|
||||
|
||||
// --- cell parsing ----------------------------------------------------------
|
||||
|
||||
/** Parse every `<mxCell>` in a model into a structured DrawioCell list. */
|
||||
export function parseCells(modelXml: string): DrawioCell[] {
|
||||
const { doc, error } = parseXml(modelXml);
|
||||
if (error) {
|
||||
throw new DrawioLintError([
|
||||
{ rule: "well-formed-xml", message: error, position: firstLineCol(error) },
|
||||
]);
|
||||
}
|
||||
const cells: DrawioCell[] = [];
|
||||
const els = doc.getElementsByTagName("mxCell");
|
||||
for (let i = 0; i < els.length; i++) {
|
||||
cells.push(readCell(els[i]));
|
||||
}
|
||||
return cells;
|
||||
}
|
||||
|
||||
function readCell(el: any): DrawioCell {
|
||||
const style = el.getAttribute("style") || "";
|
||||
const parsed = parseStyle(style);
|
||||
const geoEl = firstChildByTag(el, "mxGeometry");
|
||||
const geometry: DrawioGeometry = geoEl
|
||||
? {
|
||||
x: num(geoEl.getAttribute("x")),
|
||||
y: num(geoEl.getAttribute("y")),
|
||||
width: num(geoEl.getAttribute("width")),
|
||||
height: num(geoEl.getAttribute("height")),
|
||||
relative: geoEl.getAttribute("relative") === "1",
|
||||
hasGeometry: true,
|
||||
}
|
||||
: { relative: false, hasGeometry: false };
|
||||
return {
|
||||
id: el.getAttribute("id") ?? "",
|
||||
parent: el.getAttribute("parent") ?? undefined,
|
||||
source: el.getAttribute("source") ?? undefined,
|
||||
target: el.getAttribute("target") ?? undefined,
|
||||
vertex: el.getAttribute("vertex") === "1",
|
||||
edge: el.getAttribute("edge") === "1",
|
||||
value: el.getAttribute("value") ?? "",
|
||||
style,
|
||||
styleMap: parsed.map,
|
||||
baseStyle: parsed.baseStyle,
|
||||
geometry,
|
||||
};
|
||||
}
|
||||
|
||||
function firstChildByTag(el: any, tag: string): any {
|
||||
for (let i = 0; i < el.childNodes.length; i++) {
|
||||
const c = el.childNodes[i];
|
||||
if (c.nodeType === 1 && c.tagName === tag) return c;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function firstLineCol(msg: string): string | undefined {
|
||||
const m = /^(\d+:\d+)/.exec(msg);
|
||||
return m ? m[1] : undefined;
|
||||
}
|
||||
|
||||
// --- bounding box ----------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Absolute bounding box of the diagram from its vertex geometries. Container
|
||||
* children are relative, so absolute positions are resolved along the parent
|
||||
* chain before taking the extent. Falls back to a default canvas when empty.
|
||||
*/
|
||||
export function computeBBox(cells: DrawioCell[]): DrawioBBox {
|
||||
const byId = new Map(cells.map((c) => [c.id, c]));
|
||||
let maxX = 0;
|
||||
let maxY = 0;
|
||||
let any = false;
|
||||
for (const c of cells) {
|
||||
if (!c.vertex || !c.geometry.hasGeometry) continue;
|
||||
const g = c.geometry;
|
||||
if (g.width == null || g.height == null) continue;
|
||||
const { x, y } = absolutePos(c, byId);
|
||||
maxX = Math.max(maxX, x + g.width);
|
||||
maxY = Math.max(maxY, y + g.height);
|
||||
any = true;
|
||||
}
|
||||
if (!any) return { width: 300, height: 200 };
|
||||
// A small margin so borders/labels are not clipped at the edge.
|
||||
return { width: Math.ceil(maxX) + 20, height: Math.ceil(maxY) + 20 };
|
||||
}
|
||||
|
||||
/** Absolute (x,y) of a vertex, following its parent chain (containers). */
|
||||
export function absolutePos(
|
||||
cell: DrawioCell,
|
||||
byId: Map<string, DrawioCell>,
|
||||
): { x: number; y: number } {
|
||||
let x = cell.geometry.x ?? 0;
|
||||
let y = cell.geometry.y ?? 0;
|
||||
const seen = new Set<string>([cell.id]);
|
||||
let parentId = cell.parent;
|
||||
while (parentId && !seen.has(parentId)) {
|
||||
seen.add(parentId);
|
||||
const p = byId.get(parentId);
|
||||
// Sentinels (0/1) carry no geometry; stop there.
|
||||
if (!p || !p.vertex || !p.geometry.hasGeometry) break;
|
||||
x += p.geometry.x ?? 0;
|
||||
y += p.geometry.y ?? 0;
|
||||
parentId = p.parent;
|
||||
}
|
||||
return { x, y };
|
||||
}
|
||||
|
||||
// --- linter ----------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Run every deterministic pre-write rule over a full mxGraphModel string. On any
|
||||
* violation it throws a DrawioLintError carrying one issue per violation, each
|
||||
* with the offending cellId + position. Returns the parsed cells on success.
|
||||
*/
|
||||
export function lintModel(modelXml: string): {
|
||||
cells: DrawioCell[];
|
||||
warnings: string[];
|
||||
} {
|
||||
const issues: DrawioLintIssue[] = [];
|
||||
const warnings: string[] = [];
|
||||
|
||||
// Rule: no XML comments. Checked on the raw string (a comment survives DOM
|
||||
// parsing as a comment node, but the intent is to reject them outright — they
|
||||
// routinely wrap "TODO" cruft that breaks downstream tooling).
|
||||
if (modelXml.includes("<!--")) {
|
||||
issues.push({
|
||||
rule: "no-comments",
|
||||
message: "XML comments (<!-- -->) are not allowed in diagram XML",
|
||||
});
|
||||
}
|
||||
|
||||
// Rule: value escaping + literal newline. Scan raw <mxCell> tags so the error
|
||||
// can name the cell id even when the whole document is otherwise malformed.
|
||||
scanRawValues(modelXml, issues);
|
||||
|
||||
// Well-formedness — everything below needs a parsed DOM.
|
||||
const { doc, error } = parseXml(modelXml);
|
||||
if (error) {
|
||||
issues.push({
|
||||
rule: "well-formed-xml",
|
||||
message: error,
|
||||
position: firstLineCol(error),
|
||||
});
|
||||
throw new DrawioLintError(issues);
|
||||
}
|
||||
|
||||
const root = doc.documentElement;
|
||||
if (!root || root.tagName !== "mxGraphModel") {
|
||||
issues.push({
|
||||
rule: "structure",
|
||||
message: `root element must be <mxGraphModel>, got <${root ? root.tagName : "?"}>`,
|
||||
});
|
||||
throw new DrawioLintError(issues);
|
||||
}
|
||||
if (!firstChildByTag(root, "root")) {
|
||||
issues.push({
|
||||
rule: "structure",
|
||||
message: "<mxGraphModel> must contain a <root> element",
|
||||
});
|
||||
throw new DrawioLintError(issues);
|
||||
}
|
||||
|
||||
const cells = parseCells(modelXml);
|
||||
const ids = new Set<string>();
|
||||
|
||||
// Rule: sentinel cells id="0" and id="1"(parent="0").
|
||||
const cell0 = cells.find((c) => c.id === "0");
|
||||
const cell1 = cells.find((c) => c.id === "1");
|
||||
if (!cell0) {
|
||||
issues.push({
|
||||
rule: "sentinel-cells",
|
||||
message: 'missing the root sentinel cell <mxCell id="0"/>',
|
||||
cellId: "0",
|
||||
});
|
||||
}
|
||||
if (!cell1) {
|
||||
issues.push({
|
||||
rule: "sentinel-cells",
|
||||
message: 'missing the layer sentinel cell <mxCell id="1" parent="0"/>',
|
||||
cellId: "1",
|
||||
});
|
||||
} else if (cell1.parent !== "0") {
|
||||
issues.push({
|
||||
rule: "sentinel-cells",
|
||||
message: 'the layer sentinel <mxCell id="1"> must have parent="0"',
|
||||
cellId: "1",
|
||||
});
|
||||
}
|
||||
|
||||
cells.forEach((c, index) => {
|
||||
const pos = `cell #${index}`;
|
||||
const isSentinel = c.id === "0" || c.id === "1";
|
||||
|
||||
// Rule: unique, non-empty ids; user cells must not reuse 0/1.
|
||||
if (c.id === "") {
|
||||
issues.push({ rule: "cell-id", message: "cell has an empty id", position: pos });
|
||||
} else if (ids.has(c.id)) {
|
||||
issues.push({
|
||||
rule: "duplicate-id",
|
||||
message: `duplicate cell id "${c.id}"`,
|
||||
cellId: c.id,
|
||||
position: pos,
|
||||
});
|
||||
}
|
||||
ids.add(c.id);
|
||||
|
||||
if (isSentinel) return; // sentinels are exempt from the shape rules below
|
||||
|
||||
// Rule: vertex XOR edge (a cell may be neither: groups/containers).
|
||||
if (c.vertex && c.edge) {
|
||||
issues.push({
|
||||
rule: "vertex-edge-exclusive",
|
||||
message: 'a cell cannot be both vertex="1" and edge="1"',
|
||||
cellId: c.id,
|
||||
position: pos,
|
||||
});
|
||||
}
|
||||
|
||||
// Rule: every edge has a child <mxGeometry as="geometry"/>.
|
||||
if (c.edge && !c.geometry.hasGeometry) {
|
||||
issues.push({
|
||||
rule: "edge-geometry",
|
||||
message:
|
||||
'edge is missing its child <mxGeometry relative="1" as="geometry"/> — it will not render',
|
||||
cellId: c.id,
|
||||
position: pos,
|
||||
});
|
||||
}
|
||||
|
||||
// Rule: edge endpoints resolve to existing ids.
|
||||
if (c.edge) {
|
||||
for (const end of ["source", "target"] as const) {
|
||||
const ref = c[end];
|
||||
if (ref != null && ref !== "" && !cellExists(cells, ref)) {
|
||||
issues.push({
|
||||
rule: "edge-endpoint",
|
||||
message: `edge ${end} "${ref}" does not resolve to any cell`,
|
||||
cellId: c.id,
|
||||
position: pos,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Rule: parent must exist.
|
||||
if (c.parent != null && c.parent !== "" && !cellExists(cells, c.parent)) {
|
||||
issues.push({
|
||||
rule: "parent-exists",
|
||||
message: `parent "${c.parent}" does not resolve to any cell`,
|
||||
cellId: c.id,
|
||||
position: pos,
|
||||
});
|
||||
}
|
||||
|
||||
// Rule: style parses as key=value; pairs.
|
||||
if (c.style !== "") {
|
||||
const parsed = parseStyle(c.style);
|
||||
if (parsed.badSegment !== undefined) {
|
||||
issues.push({
|
||||
rule: "style-format",
|
||||
message: `malformed style segment "${parsed.badSegment}" (expected key=value)`,
|
||||
cellId: c.id,
|
||||
position: pos,
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
if (issues.length > 0) throw new DrawioLintError(issues);
|
||||
return { cells, warnings };
|
||||
}
|
||||
|
||||
function cellExists(cells: DrawioCell[], id: string): boolean {
|
||||
return cells.some((c) => c.id === id);
|
||||
}
|
||||
|
||||
/**
|
||||
* Raw-string scan of every `value="…"`/`value='…'` on an mxCell tag. Catches an
|
||||
* unescaped `&`/`<`/`>` and a literal newline character inside a value, keyed to
|
||||
* the cell's id. Runs before DOM parsing so a value bug is reported with its
|
||||
* cellId even when the document is otherwise malformed.
|
||||
*/
|
||||
function scanRawValues(xml: string, issues: DrawioLintIssue[]): void {
|
||||
const tagRe = /<mxCell\b([^>]*?)\/?>/g;
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = tagRe.exec(xml)) !== null) {
|
||||
const attrs = m[1];
|
||||
const idM = /\bid\s*=\s*"([^"]*)"/.exec(attrs);
|
||||
const cellId = idM ? idM[1] : undefined;
|
||||
const valM = /\bvalue\s*=\s*"([^"]*)"/.exec(attrs) || /\bvalue\s*=\s*'([^']*)'/.exec(attrs);
|
||||
if (!valM) continue;
|
||||
const raw = valM[1];
|
||||
// Literal newline (0x0A / 0x0D) inside the attribute value.
|
||||
if (/[\n\r]/.test(raw)) {
|
||||
issues.push({
|
||||
rule: "value-newline",
|
||||
message:
|
||||
"value contains a literal newline; use 
 (or <br> with html=1) instead",
|
||||
cellId,
|
||||
});
|
||||
}
|
||||
// Unescaped '<' or '>' inside a value.
|
||||
if (raw.includes("<") || raw.includes(">")) {
|
||||
issues.push({
|
||||
rule: "value-escaping",
|
||||
message: "value contains an unescaped '<' or '>'; use < / >",
|
||||
cellId,
|
||||
});
|
||||
}
|
||||
// '&' that does not begin a valid entity.
|
||||
const badAmp = /&(?!(amp|lt|gt|quot|apos|#[0-9]+|#x[0-9a-fA-F]+);)/.test(raw);
|
||||
if (badAmp) {
|
||||
issues.push({
|
||||
rule: "value-escaping",
|
||||
message: "value contains an unescaped '&'; use &",
|
||||
cellId,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// --- input normalization + prepare -----------------------------------------
|
||||
|
||||
/**
|
||||
* Normalize an accepted tool input into a full mxGraphModel string:
|
||||
* - a bare `<mxGraphModel>` is used as-is;
|
||||
* - an `<mxfile>` is decoded to its first page's model;
|
||||
* - a list of `<mxCell>` is wrapped with the mxGraphModel/root envelope and
|
||||
* the sentinel cells (id=0, id=1 parent=0) are added when absent.
|
||||
*/
|
||||
export function normalizeInput(inputXml: string): string {
|
||||
let xml = inputXml.trim();
|
||||
// Strip an optional XML prolog.
|
||||
if (xml.startsWith("<?xml")) {
|
||||
const end = xml.indexOf("?>");
|
||||
if (end !== -1) xml = xml.slice(end + 2).trim();
|
||||
}
|
||||
if (xml.startsWith("<mxfile")) {
|
||||
return decodeDrawioFileToModel(xml);
|
||||
}
|
||||
if (xml.startsWith("<mxGraphModel")) {
|
||||
return xml;
|
||||
}
|
||||
if (xml.includes("<mxCell")) {
|
||||
return wrapCellFragment(xml);
|
||||
}
|
||||
throw new DrawioLintError([
|
||||
{
|
||||
rule: "unrecognized-input",
|
||||
message:
|
||||
"input must be a <mxGraphModel>, an <mxfile>, or a list of <mxCell> elements",
|
||||
},
|
||||
]);
|
||||
}
|
||||
|
||||
function wrapCellFragment(fragment: string): string {
|
||||
// Validate the fragment is well-formed (wrapped so a bare list parses) and
|
||||
// discover which sentinels are already present.
|
||||
const { doc, error } = parseXml(`<root>${fragment}</root>`);
|
||||
if (error) {
|
||||
throw new DrawioLintError([
|
||||
{
|
||||
rule: "well-formed-xml",
|
||||
message: error,
|
||||
position: firstLineCol(error),
|
||||
},
|
||||
]);
|
||||
}
|
||||
const existing = new Set<string>();
|
||||
const els = doc.getElementsByTagName("mxCell");
|
||||
for (let i = 0; i < els.length; i++) {
|
||||
existing.add(els[i].getAttribute("id") ?? "");
|
||||
}
|
||||
let prefix = "";
|
||||
if (!existing.has("0")) prefix += '<mxCell id="0"/>';
|
||||
if (!existing.has("1")) prefix += '<mxCell id="1" parent="0"/>';
|
||||
return `<mxGraphModel ${DEFAULT_MODEL_ATTRS}><root>${prefix}${fragment}</root></mxGraphModel>`;
|
||||
}
|
||||
|
||||
export interface PreparedModel {
|
||||
/** Canonical (normalized) mxGraphModel XML that gets written. */
|
||||
modelXml: string;
|
||||
cells: DrawioCell[];
|
||||
bbox: DrawioBBox;
|
||||
/** Number of user cells (excludes the id=0/id=1 sentinels). */
|
||||
cellCount: number;
|
||||
warnings: string[];
|
||||
hash: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Full pre-write pipeline for create/update: normalize the input into a model,
|
||||
* lint it (throws DrawioLintError on any violation), then compute the canonical
|
||||
* form, bounding box, cell count and hash. Never touches the network.
|
||||
*/
|
||||
export function prepareModel(inputXml: string): PreparedModel {
|
||||
const rawModel = normalizeInput(inputXml);
|
||||
const { cells, warnings } = lintModel(rawModel);
|
||||
const modelXml = normalizeXml(rawModel);
|
||||
const bbox = computeBBox(cells);
|
||||
const cellCount = cells.filter((c) => c.id !== "0" && c.id !== "1").length;
|
||||
return {
|
||||
modelXml,
|
||||
cells,
|
||||
bbox,
|
||||
cellCount,
|
||||
warnings,
|
||||
hash: mxHash(modelXml),
|
||||
};
|
||||
}
|
||||
|
||||
/** Cell count of a decoded model (user cells only) — used by drawio_get meta. */
|
||||
export function countUserCells(modelXml: string): number {
|
||||
return parseCells(modelXml).filter((c) => c.id !== "0" && c.id !== "1").length;
|
||||
}
|
||||
@@ -12,7 +12,11 @@
|
||||
* re-import for small wording fixes.
|
||||
*/
|
||||
|
||||
import { stripInlineMarkdown, stripBalancedWrappers } from "./text-normalize.js";
|
||||
import {
|
||||
stripInlineMarkdown,
|
||||
stripBalancedWrappers,
|
||||
closestBlockHint,
|
||||
} from "./text-normalize.js";
|
||||
|
||||
export interface TextEdit {
|
||||
find: string;
|
||||
@@ -381,29 +385,9 @@ export function applyTextEdits(
|
||||
} else {
|
||||
// Append a bounded "closest text" hint: find the FIRST block that
|
||||
// contains the longest whitespace-delimited token (>= 3 chars) of the
|
||||
// (stripped, then raw) locator, and quote that block's plain text.
|
||||
reason = "text not found in the document.";
|
||||
const tokenSource = stripped.length > 0 ? stripped : edit.find;
|
||||
const longestToken = tokenSource
|
||||
.split(/\s+/)
|
||||
.filter((t) => t.length >= 3)
|
||||
.sort((a, b) => b.length - a.length)[0];
|
||||
if (longestToken) {
|
||||
const hitBlock = blockPlain.find((plain) =>
|
||||
plain.includes(longestToken),
|
||||
);
|
||||
if (hitBlock) {
|
||||
// Truncate by code point (spread iterates by code point) so a
|
||||
// surrogate pair is never split; append the ellipsis only when the
|
||||
// text was actually longer than the limit.
|
||||
const points = [...hitBlock];
|
||||
const snippet =
|
||||
points.length > 120
|
||||
? points.slice(0, 120).join("") + "…"
|
||||
: hitBlock;
|
||||
reason += ` Closest block text: "${snippet}".`;
|
||||
}
|
||||
}
|
||||
// (stripped, then raw) locator, and quote that block's plain text. Shared
|
||||
// with create_comment via closestBlockHint so both give the same hint.
|
||||
reason = "text not found in the document." + closestBlockHint(blockPlain, edit.find);
|
||||
}
|
||||
failed.push({ find: edit.find, reason });
|
||||
continue;
|
||||
|
||||
Vendored
+61
@@ -0,0 +1,61 @@
|
||||
// Minimal ambient type declaration for `pako` (no @types/pako is installed and
|
||||
// pako 2.x ships no bundled .d.ts). We only use the raw-deflate codec to read
|
||||
// draw.io's compressed `<diagram>` payload, so declare just that surface.
|
||||
declare module "pako" {
|
||||
interface RawOptions {
|
||||
/** When "string", the result is returned as a (binary/UTF-8) string. */
|
||||
to?: "string";
|
||||
/** Raw-deflate window bits; draw.io uses raw deflate (no zlib header). */
|
||||
windowBits?: number;
|
||||
level?: number;
|
||||
}
|
||||
|
||||
/** Raw-inflate (windowBits: -15). `to:"string"` yields a string. */
|
||||
export function inflateRaw(
|
||||
data: Uint8Array | ArrayBuffer | number[],
|
||||
options: RawOptions & { to: "string" },
|
||||
): string;
|
||||
export function inflateRaw(
|
||||
data: Uint8Array | ArrayBuffer | number[],
|
||||
options?: RawOptions,
|
||||
): Uint8Array;
|
||||
|
||||
/** Raw-deflate (windowBits: -15). Used only by tests to build fixtures. */
|
||||
export function deflateRaw(
|
||||
data: Uint8Array | string,
|
||||
options?: RawOptions,
|
||||
): Uint8Array;
|
||||
|
||||
interface InflateStreamOptions {
|
||||
to?: "string";
|
||||
windowBits?: number;
|
||||
/** Raw deflate (no zlib header) — equivalent to windowBits: -15. */
|
||||
raw?: boolean;
|
||||
chunkSize?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Streaming inflate. We use it to bound the decompressed size: `onData` is
|
||||
* invoked per output chunk, letting us abort a decompression bomb before the
|
||||
* full output is materialised.
|
||||
*/
|
||||
export class Inflate {
|
||||
constructor(options?: InflateStreamOptions);
|
||||
onData: (chunk: string | Uint8Array) => void;
|
||||
onEnd: (status: number) => void;
|
||||
push(
|
||||
data: Uint8Array | ArrayBuffer | number[] | string,
|
||||
flushMode?: boolean | number,
|
||||
): boolean;
|
||||
result: string | Uint8Array;
|
||||
err: number;
|
||||
msg: string;
|
||||
}
|
||||
|
||||
const _default: {
|
||||
inflateRaw: typeof inflateRaw;
|
||||
deflateRaw: typeof deflateRaw;
|
||||
Inflate: typeof Inflate;
|
||||
};
|
||||
export default _default;
|
||||
}
|
||||
@@ -114,3 +114,37 @@ export function stripInlineMarkdown(s: string): string {
|
||||
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a bounded "closest text" hint for an anchor/find MISS, shared by
|
||||
* edit_page_text (json-edit) and create_comment (client) so both surface the
|
||||
* same self-correction affordance.
|
||||
*
|
||||
* Take the longest whitespace-delimited token (>= 3 chars) of the locator
|
||||
* (markdown-stripped first, so `**bold**` contributes `bold`), find the FIRST
|
||||
* of `blockTexts` that contains it, and return ` Closest block text: "…".` with
|
||||
* the block quoted (truncated to 120 code points + ellipsis). Returns "" when
|
||||
* no token qualifies or no block contains it, so the caller can append it
|
||||
* unconditionally.
|
||||
*/
|
||||
export function closestBlockHint(
|
||||
blockTexts: string[],
|
||||
locator: string,
|
||||
): string {
|
||||
if (typeof locator !== "string" || locator.length === 0) return "";
|
||||
const stripped = stripInlineMarkdown(locator);
|
||||
const tokenSource = stripped.length > 0 ? stripped : locator;
|
||||
const longestToken = tokenSource
|
||||
.split(/\s+/)
|
||||
.filter((t) => t.length >= 3)
|
||||
.sort((a, b) => b.length - a.length)[0];
|
||||
if (!longestToken) return "";
|
||||
const hitBlock = blockTexts.find((plain) => plain.includes(longestToken));
|
||||
if (!hitBlock) return "";
|
||||
// Truncate by code point (spread iterates by code point) so a surrogate pair
|
||||
// is never split; append the ellipsis only when the text was actually longer.
|
||||
const points = [...hitBlock];
|
||||
const snippet =
|
||||
points.length > 120 ? points.slice(0, 120).join("") + "…" : hitBlock;
|
||||
return ` Closest block text: "${snippet}".`;
|
||||
}
|
||||
|
||||
@@ -771,9 +771,13 @@ export const SHARED_TOOL_SPECS = {
|
||||
'The comment is anchored inline to the given exact `selection` text ' +
|
||||
'(which gets highlighted); page-level comments are NOT supported. A ' +
|
||||
'new top-level comment REQUIRES a `selection`. Replies inherit the ' +
|
||||
"parent's anchor and take no selection. If the call fails with a " +
|
||||
'"selection not found" error, retry with a corrected EXACT selection ' +
|
||||
'copied verbatim from a single paragraph/block. You may also attach a ' +
|
||||
"parent's anchor and take no selection. Always COPY the `selection` " +
|
||||
'VERBATIM from get_page / search_in_page output — do NOT quote it from ' +
|
||||
'memory (stale-memory quoting is the top cause of anchor misses). If the ' +
|
||||
'call fails with a "selection not found" error, the error quotes the ' +
|
||||
"closest block text (or says the selection spans multiple blocks); retry " +
|
||||
"with a corrected EXACT selection copied verbatim from a single " +
|
||||
'paragraph/block. You may also attach a ' +
|
||||
'`suggestedText` proposing a replacement for the `selection` (a human ' +
|
||||
'applies it from the UI); when set, the `selection` must occur exactly ' +
|
||||
'once in the page. Reversible via the comment UI.',
|
||||
@@ -1115,4 +1119,112 @@ export const SHARED_TOOL_SPECS = {
|
||||
alt: z.string().optional(),
|
||||
}),
|
||||
},
|
||||
|
||||
// --- draw.io diagrams (issue #423, stage 1) ---
|
||||
|
||||
drawioGet: {
|
||||
mcpName: 'drawio_get',
|
||||
inAppKey: 'drawioGet',
|
||||
description:
|
||||
'Read a draw.io diagram on a page as mxGraph XML (default) or as its raw ' +
|
||||
'`.drawio.svg`. `node` is the drawio node\'s attrs.id (from get_outline / ' +
|
||||
'get_page_json) or "#<index>" for a top-level block. Returns the decoded ' +
|
||||
'mxGraphModel XML plus meta { attachmentId, title, width, height, ' +
|
||||
'cellCount, hash }. `hash` is the optimistic-lock key you MUST pass back ' +
|
||||
'as baseHash to drawio_update. Diagrams a human saved from the editor ' +
|
||||
'(including draw.io\'s compressed format) decode losslessly.',
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
'drawioGet — read a draw.io diagram as mxGraph XML (+ hash for updates).',
|
||||
buildShape: (z) => ({
|
||||
pageId: z.string().min(1),
|
||||
node: z
|
||||
.string()
|
||||
.min(1)
|
||||
.describe('The drawio node attrs.id, or "#<index>" for a top-level block.'),
|
||||
format: z
|
||||
.enum(['xml', 'svg'])
|
||||
.optional()
|
||||
.describe('"xml" (default) for mxGraph XML, or "svg" for the raw .drawio.svg.'),
|
||||
}),
|
||||
},
|
||||
|
||||
drawioCreate: {
|
||||
mcpName: 'drawio_create',
|
||||
inAppKey: 'drawioCreate',
|
||||
description:
|
||||
'Create a draw.io diagram from mxGraph XML and insert it as a diagram ' +
|
||||
'block. `xml` is a bare `<mxGraphModel>` OR a list of `<mxCell>` elements ' +
|
||||
'(the server wraps it and adds the id=0 / id=1 sentinel cells). The XML is ' +
|
||||
'LINTED first (well-formedness, sentinel cells, unique ids, vertex XOR ' +
|
||||
'edge, every edge has a child <mxGeometry as="geometry"/>, edge ' +
|
||||
'source/target and every parent resolve, style parses, no XML comments, ' +
|
||||
'value escaping) — a violation returns a structured error naming the rule ' +
|
||||
'and cellId so you can fix and retry. `where` positions the block like ' +
|
||||
'insert_node: position before/after (with exactly one of anchorNodeId or ' +
|
||||
'anchorText) or append. Returns { nodeId, attachmentId, warnings }. The ' +
|
||||
'returned `nodeId` is an index-based "#<index>" handle (drawio nodes carry ' +
|
||||
'no attrs.id): it addresses the new top-level block and can be fed straight ' +
|
||||
'back into drawio_get / drawio_update for THIS document. It is positional, ' +
|
||||
'so if you add or remove blocks before it, re-resolve via get_outline. The ' +
|
||||
'diagram is editable in the draw.io editor and can be re-read with ' +
|
||||
'drawio_get.',
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
'drawioCreate — create a draw.io diagram from mxGraph XML and insert it.',
|
||||
buildShape: (z) => ({
|
||||
pageId: z.string().min(1),
|
||||
xml: z
|
||||
.string()
|
||||
.min(1)
|
||||
.describe(
|
||||
'mxGraph XML: a bare <mxGraphModel> or a list of <mxCell> elements.',
|
||||
),
|
||||
position: z
|
||||
.enum(['before', 'after', 'append'])
|
||||
.describe('Where to insert relative to the anchor.'),
|
||||
anchorNodeId: z
|
||||
.string()
|
||||
.optional()
|
||||
.describe('Anchor block id (for before/after).'),
|
||||
anchorText: z
|
||||
.string()
|
||||
.optional()
|
||||
.describe('Anchor text fragment (for before/after).'),
|
||||
title: z.string().optional().describe('Optional diagram title.'),
|
||||
}),
|
||||
},
|
||||
|
||||
drawioUpdate: {
|
||||
mcpName: 'drawio_update',
|
||||
inAppKey: 'drawioUpdate',
|
||||
description:
|
||||
'Replace a draw.io diagram\'s content with new mxGraph XML (same lint ' +
|
||||
'pipeline as drawio_create). `baseHash` is MANDATORY: pass the hash from ' +
|
||||
'the drawio_get you based the edit on. If the diagram changed since ' +
|
||||
'(a human or another agent edited it) the hash mismatches and the update ' +
|
||||
'is refused with a conflict error — re-read with drawio_get and retry. On ' +
|
||||
'success it overwrites the diagram attachment and updates the node ' +
|
||||
'width/height. `node` is the drawio node attrs.id or "#<index>".',
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
'drawioUpdate — replace a draw.io diagram (optimistic-locked by baseHash).',
|
||||
buildShape: (z) => ({
|
||||
pageId: z.string().min(1),
|
||||
node: z
|
||||
.string()
|
||||
.min(1)
|
||||
.describe('The drawio node attrs.id, or "#<index>" for a top-level block.'),
|
||||
xml: z
|
||||
.string()
|
||||
.min(1)
|
||||
.describe(
|
||||
'New mxGraph XML: a bare <mxGraphModel> or a list of <mxCell> elements.',
|
||||
),
|
||||
baseHash: z
|
||||
.string()
|
||||
.min(1)
|
||||
.describe('The meta.hash from the drawio_get this edit is based on.'),
|
||||
}),
|
||||
},
|
||||
} satisfies Record<string, SharedToolSpec>;
|
||||
|
||||
@@ -548,3 +548,94 @@ test("suggestedText: the stored selection is the doc's RAW typographic substring
|
||||
);
|
||||
assert.equal(createPayload.suggestedText, "goodbye");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// 8) #408: a not-found selection error QUOTES the closest block text so the
|
||||
// model can self-correct instead of blind-retrying.
|
||||
// -----------------------------------------------------------------------------
|
||||
test("a not-found selection error includes a 'Closest block text' hint", async () => {
|
||||
let createCalls = 0;
|
||||
const { baseURL } = await spawn(async (req, res) => {
|
||||
await readBody(req);
|
||||
if (req.url === "/api/auth/login") {
|
||||
sendJson(res, 200, { success: true }, {
|
||||
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/pages/info") {
|
||||
sendJson(res, 200, {
|
||||
data: {
|
||||
id: "page-1",
|
||||
content: {
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "The quick brown fox jumps" }] },
|
||||
],
|
||||
},
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/comments/create") {
|
||||
createCalls++;
|
||||
sendJson(res, 200, { data: { id: "should-not-happen" } });
|
||||
return;
|
||||
}
|
||||
sendJson(res, 404, { message: "not found" });
|
||||
});
|
||||
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
await assert.rejects(
|
||||
() => client.createComment("page-1", "body", "inline", "quick brown cat"),
|
||||
/Closest block text: "The quick brown fox jumps"/,
|
||||
"a not-found selection must quote the closest block text",
|
||||
);
|
||||
assert.equal(createCalls, 0, "/comments/create must NOT be called on a miss");
|
||||
});
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// 9) #408: a selection that straddles two blocks gets the explicit
|
||||
// "spans multiple blocks" message instead of a bare not-found.
|
||||
// -----------------------------------------------------------------------------
|
||||
test("a selection spanning multiple blocks gets the explicit spans-multiple-blocks message", async () => {
|
||||
let createCalls = 0;
|
||||
const { baseURL } = await spawn(async (req, res) => {
|
||||
await readBody(req);
|
||||
if (req.url === "/api/auth/login") {
|
||||
sendJson(res, 200, { success: true }, {
|
||||
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/pages/info") {
|
||||
sendJson(res, 200, {
|
||||
data: {
|
||||
id: "page-1",
|
||||
content: {
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "the quick brown" }] },
|
||||
{ type: "paragraph", content: [{ type: "text", text: "fox jumps over" }] },
|
||||
],
|
||||
},
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/comments/create") {
|
||||
createCalls++;
|
||||
sendJson(res, 200, { data: { id: "should-not-happen" } });
|
||||
return;
|
||||
}
|
||||
sendJson(res, 404, { message: "not found" });
|
||||
});
|
||||
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
await assert.rejects(
|
||||
() => client.createComment("page-1", "body", "inline", "brown fox"),
|
||||
/spans multiple blocks/,
|
||||
"a cross-block selection must report the spans-multiple-blocks hint",
|
||||
);
|
||||
assert.equal(createCalls, 0, "/comments/create must NOT be called on a miss");
|
||||
});
|
||||
|
||||
@@ -0,0 +1,467 @@
|
||||
// Contract tests for the drawio_get / drawio_create / drawio_update client
|
||||
// methods (issue #423). Follows the repo's seam-override pattern (see
|
||||
// full-doc-write-canonicalize.test.mjs): a DocmostClient subclass stubs the I/O
|
||||
// seams (auth, collab token, page read, attachment upload/fetch, the mutatePage
|
||||
// write) so the tool logic is exercised without a live Docmost or collab socket.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import pako from "pako";
|
||||
import { DocmostClient } from "../../build/client.js";
|
||||
import {
|
||||
buildDrawioSvg,
|
||||
encodeDrawioFile,
|
||||
normalizeXml,
|
||||
mxHash,
|
||||
decodeDrawioSvg,
|
||||
} from "../../build/lib/drawio-xml.js";
|
||||
|
||||
const MODEL =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" value="Hi" style="rounded=1;" vertex="1" parent="1">' +
|
||||
'<mxGeometry x="20" y="20" width="120" height="60" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
|
||||
// Build a Docmost-style `.drawio.svg` (base64 content) for a model.
|
||||
function svgFor(model) {
|
||||
return buildDrawioSvg(normalizeXml(model), "<g/>", { width: 200, height: 120 });
|
||||
}
|
||||
|
||||
// Build a human/compressed-export `.drawio.svg` (base64 content wrapping a
|
||||
// compressed <diagram> payload), mimicking a diagram a person saved.
|
||||
function compressedSvgFor(model) {
|
||||
const compressed = Buffer.from(
|
||||
pako.deflateRaw(encodeURIComponent(normalizeXml(model))),
|
||||
).toString("base64");
|
||||
const file = `<mxfile host="Electron"><diagram id="a" name="Page-1">${compressed}</diagram></mxfile>`;
|
||||
const content = Buffer.from(file, "utf-8").toString("base64");
|
||||
return `<svg xmlns="http://www.w3.org/2000/svg" content="${content}"><image href="x"/></svg>`;
|
||||
}
|
||||
|
||||
// The vendored `drawio` node schema (diagramAttributes) declares ONLY these
|
||||
// attributes; PMNode.fromJSON drops anything else on save. Mirror that here so
|
||||
// the mock write path behaves like the real one — in particular, a block `id`
|
||||
// set on a drawio node does NOT survive the save, so a handle keyed on it is
|
||||
// un-resolvable. This is exactly what the production bug (issue #423 Fix 1) was.
|
||||
const DRAWIO_SCHEMA_ATTRS = new Set([
|
||||
"src",
|
||||
"title",
|
||||
"alt",
|
||||
"width",
|
||||
"height",
|
||||
"size",
|
||||
"aspectRatio",
|
||||
"align",
|
||||
"attachmentId",
|
||||
]);
|
||||
|
||||
function applyDrawioSchemaDrop(node) {
|
||||
if (!node || typeof node !== "object") return;
|
||||
if (node.type === "drawio" && node.attrs && typeof node.attrs === "object") {
|
||||
for (const key of Object.keys(node.attrs)) {
|
||||
if (!DRAWIO_SCHEMA_ATTRS.has(key)) delete node.attrs[key];
|
||||
}
|
||||
}
|
||||
if (Array.isArray(node.content)) for (const c of node.content) applyDrawioSchemaDrop(c);
|
||||
}
|
||||
|
||||
function makeClient({ pageDoc, attachmentSvg } = {}) {
|
||||
const calls = { uploads: [], mutations: [] };
|
||||
class TestClient extends DocmostClient {
|
||||
async ensureAuthenticated() {}
|
||||
async getCollabTokenWithReauth() {
|
||||
return "collab-token";
|
||||
}
|
||||
async resolvePageId(pageId) {
|
||||
return `uuid-${pageId}`;
|
||||
}
|
||||
async getPageRaw(pageId) {
|
||||
return {
|
||||
id: pageId,
|
||||
slugId: "s",
|
||||
title: "P",
|
||||
spaceId: "sp",
|
||||
content: pageDoc ?? { type: "doc", content: [] },
|
||||
};
|
||||
}
|
||||
async uploadAttachmentBuffer(pageId, buffer, fileName, mime) {
|
||||
const id = `att-${calls.uploads.length + 1}`;
|
||||
calls.uploads.push({ pageId, fileName, mime, svg: buffer.toString("utf-8") });
|
||||
return { id, fileName, fileSize: buffer.length };
|
||||
}
|
||||
async fetchAttachmentText(src) {
|
||||
return attachmentSvg;
|
||||
}
|
||||
mutatePage(pageId, token, apiUrl, transform) {
|
||||
// Run the transform against a clone of the source doc, capture the result.
|
||||
const clone = structuredClone(pageDoc ?? { type: "doc", content: [] });
|
||||
const doc = transform(clone);
|
||||
// Mirror the real schema: unknown drawio attrs (e.g. a block `id`) are
|
||||
// dropped on save, so callers can never rely on them to address the node.
|
||||
if (doc) applyDrawioSchemaDrop(doc);
|
||||
calls.mutations.push({ pageId, doc });
|
||||
return Promise.resolve({ doc, verify: { changed: doc != null } });
|
||||
}
|
||||
}
|
||||
const client = new TestClient("http://127.0.0.1:1/api", "e@x.com", "pw");
|
||||
return { client, calls };
|
||||
}
|
||||
|
||||
function findDrawio(node, acc = []) {
|
||||
if (!node || typeof node !== "object") return acc;
|
||||
if (node.type === "drawio") acc.push(node);
|
||||
if (Array.isArray(node.content)) for (const c of node.content) findDrawio(c, acc);
|
||||
return acc;
|
||||
}
|
||||
|
||||
// --- drawio_create ---------------------------------------------------------
|
||||
|
||||
test("drawio_create: lints, builds the .drawio.svg, uploads and inserts a node", async () => {
|
||||
const pageDoc = {
|
||||
type: "doc",
|
||||
content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }],
|
||||
};
|
||||
const { client, calls } = makeClient({ pageDoc });
|
||||
const res = await client.drawioCreate("page1", { position: "append" }, MODEL, "My diagram");
|
||||
|
||||
assert.equal(res.success, true);
|
||||
// The returned handle is an index-based "#<index>" ref (drawio nodes carry no
|
||||
// persisted attrs.id), addressing the appended top-level block (index 1, after
|
||||
// the existing paragraph).
|
||||
assert.equal(res.nodeId, "#1");
|
||||
assert.equal(res.attachmentId, "att-1");
|
||||
assert.equal(calls.uploads.length, 1);
|
||||
assert.equal(calls.uploads[0].fileName, "diagram.drawio.svg");
|
||||
assert.equal(calls.uploads[0].mime, "image/svg+xml");
|
||||
// The uploaded SVG carries the model back (round-trips through the decode chain).
|
||||
assert.equal(decodeDrawioSvg(calls.uploads[0].svg), normalizeXml(MODEL));
|
||||
|
||||
// A drawio node was appended with src/attachmentId/dimensions and the title.
|
||||
const drawios = findDrawio(calls.mutations[0].doc);
|
||||
assert.equal(drawios.length, 1);
|
||||
const n = drawios[0];
|
||||
// No `id` attribute is set/persisted on the node (schema has none).
|
||||
assert.equal(n.attrs.id, undefined);
|
||||
assert.equal(n.attrs.attachmentId, "att-1");
|
||||
assert.match(n.attrs.src, /^\/api\/files\/att-1\//);
|
||||
assert.ok(n.attrs.width > 0 && n.attrs.height > 0);
|
||||
assert.equal(n.attrs.title, "My diagram");
|
||||
});
|
||||
|
||||
test("drawio_create: a lint violation throws before any upload", async () => {
|
||||
const { client, calls } = makeClient({ pageDoc: { type: "doc", content: [] } });
|
||||
// Edge with no child geometry -> edge-geometry rule.
|
||||
const bad =
|
||||
'<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="3" edge="1" parent="1" source="2" target="2"/></root></mxGraphModel>';
|
||||
await assert.rejects(
|
||||
() => client.drawioCreate("page1", { position: "append" }, bad, undefined),
|
||||
/edge-geometry/,
|
||||
);
|
||||
assert.equal(calls.uploads.length, 0, "no attachment uploaded on lint failure");
|
||||
});
|
||||
|
||||
test("drawio_create: before/after requires exactly one anchor", async () => {
|
||||
const { client } = makeClient({ pageDoc: { type: "doc", content: [] } });
|
||||
await assert.rejects(
|
||||
() => client.drawioCreate("page1", { position: "before" }, MODEL),
|
||||
/exactly one of anchorNodeId or anchorText/,
|
||||
);
|
||||
});
|
||||
|
||||
// --- drawio_get ------------------------------------------------------------
|
||||
|
||||
test("drawio_get: decodes the model and returns meta with a hash", async () => {
|
||||
const pageDoc = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "drawio",
|
||||
attrs: {
|
||||
id: "d1",
|
||||
src: "/api/files/att-1/diagram.drawio.svg",
|
||||
attachmentId: "att-1",
|
||||
title: "T",
|
||||
width: 200,
|
||||
height: 120,
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
const { client } = makeClient({ pageDoc, attachmentSvg: svgFor(MODEL) });
|
||||
const res = await client.drawioGet("page1", "d1", "xml");
|
||||
assert.equal(res.content, normalizeXml(MODEL));
|
||||
assert.equal(res.meta.attachmentId, "att-1");
|
||||
assert.equal(res.meta.title, "T");
|
||||
assert.equal(res.meta.cellCount, 1);
|
||||
assert.equal(res.meta.hash, mxHash(normalizeXml(MODEL)));
|
||||
});
|
||||
|
||||
test("drawio_get: format=svg returns the raw .drawio.svg", async () => {
|
||||
const svg = svgFor(MODEL);
|
||||
const pageDoc = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "drawio", attrs: { id: "d1", src: "/api/files/att-1/x.svg", attachmentId: "att-1" } },
|
||||
],
|
||||
};
|
||||
const { client } = makeClient({ pageDoc, attachmentSvg: svg });
|
||||
const res = await client.drawioGet("page1", "d1", "svg");
|
||||
assert.equal(res.content, svg);
|
||||
});
|
||||
|
||||
test("drawio_get: reads a HUMAN-saved compressed diagram losslessly (pako)", async () => {
|
||||
const pageDoc = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "drawio", attrs: { id: "d1", src: "/api/files/att-1/x.svg", attachmentId: "att-1" } },
|
||||
],
|
||||
};
|
||||
const { client } = makeClient({ pageDoc, attachmentSvg: compressedSvgFor(MODEL) });
|
||||
const res = await client.drawioGet("page1", "d1", "xml");
|
||||
assert.equal(res.content, normalizeXml(MODEL));
|
||||
});
|
||||
|
||||
// --- drawio_update ---------------------------------------------------------
|
||||
|
||||
const UPDATED_MODEL =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" value="Changed" style="rounded=1;" vertex="1" parent="1">' +
|
||||
'<mxGeometry x="20" y="20" width="300" height="200" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
|
||||
function updatePageDoc() {
|
||||
return {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "drawio",
|
||||
attrs: {
|
||||
id: "d1",
|
||||
src: "/api/files/att-1/diagram.drawio.svg",
|
||||
attachmentId: "att-1",
|
||||
width: 200,
|
||||
height: 120,
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
test("drawio_update: stale baseHash -> conflict, no upload", async () => {
|
||||
const { client, calls } = makeClient({
|
||||
pageDoc: updatePageDoc(),
|
||||
attachmentSvg: svgFor(MODEL),
|
||||
});
|
||||
await assert.rejects(
|
||||
() => client.drawioUpdate("page1", "d1", UPDATED_MODEL, "deadbeef-stale"),
|
||||
/conflict/,
|
||||
);
|
||||
assert.equal(calls.uploads.length, 0, "no upload on conflict");
|
||||
});
|
||||
|
||||
test("drawio_update: current baseHash -> uploads new attachment and repoints node dims", async () => {
|
||||
const currentHash = mxHash(normalizeXml(MODEL));
|
||||
const { client, calls } = makeClient({
|
||||
pageDoc: updatePageDoc(),
|
||||
attachmentSvg: svgFor(MODEL),
|
||||
});
|
||||
const res = await client.drawioUpdate("page1", "d1", UPDATED_MODEL, currentHash);
|
||||
assert.equal(res.success, true);
|
||||
assert.equal(res.attachmentId, "att-1"); // fresh id from the stub sequence
|
||||
assert.equal(calls.uploads.length, 1);
|
||||
// The uploaded SVG carries the NEW model.
|
||||
assert.equal(decodeDrawioSvg(calls.uploads[0].svg), normalizeXml(UPDATED_MODEL));
|
||||
// The node was repointed with the new bounding-box dimensions:
|
||||
// vertex maxX=320,maxY=220 + the 20px preview margin -> 340 x 240.
|
||||
const n = findDrawio(calls.mutations[0].doc)[0];
|
||||
assert.equal(n.attrs.attachmentId, "att-1");
|
||||
assert.equal(n.attrs.width, 340);
|
||||
assert.equal(n.attrs.height, 240);
|
||||
// The block `id` used as the legacy resolution handle is dropped on save
|
||||
// (schema declares no `id`); the update still targeted the correct node.
|
||||
assert.equal(n.attrs.id, undefined);
|
||||
});
|
||||
|
||||
test("drawio_update: baseHash is mandatory", async () => {
|
||||
const { client } = makeClient({ pageDoc: updatePageDoc(), attachmentSvg: svgFor(MODEL) });
|
||||
await assert.rejects(
|
||||
() => client.drawioUpdate("page1", "d1", UPDATED_MODEL, ""),
|
||||
/baseHash is mandatory/,
|
||||
);
|
||||
});
|
||||
|
||||
// --- Fix 1: the create handle must resolve on the SAVED doc (no id) ---------
|
||||
|
||||
test("drawio_create -> get/update: returned #<index> handle resolves on the saved doc (id dropped)", async () => {
|
||||
// Create appends a drawio node after the existing paragraph.
|
||||
const createDoc = {
|
||||
type: "doc",
|
||||
content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }],
|
||||
};
|
||||
const create = makeClient({ pageDoc: createDoc });
|
||||
const res = await create.client.drawioCreate(
|
||||
"page1",
|
||||
{ position: "append" },
|
||||
MODEL,
|
||||
"T",
|
||||
);
|
||||
// The handle is index-based, not a block id.
|
||||
assert.equal(res.nodeId, "#1");
|
||||
|
||||
// Take the document EXACTLY as it was saved: the schema drop stripped the
|
||||
// node's id, so no id-based handle could ever resolve against it.
|
||||
const savedDoc = create.calls.mutations[0].doc;
|
||||
assert.equal(findDrawio(savedDoc)[0].attrs.id, undefined);
|
||||
|
||||
// drawio_get with the returned handle resolves the just-created node.
|
||||
const getClient = makeClient({ pageDoc: savedDoc, attachmentSvg: svgFor(MODEL) });
|
||||
const got = await getClient.client.drawioGet("page1", res.nodeId, "xml");
|
||||
assert.equal(got.nodeId, res.nodeId);
|
||||
assert.equal(got.content, normalizeXml(MODEL));
|
||||
|
||||
// drawio_update with the same handle + the hash from get repoints that node.
|
||||
const upClient = makeClient({ pageDoc: savedDoc, attachmentSvg: svgFor(MODEL) });
|
||||
const upd = await upClient.client.drawioUpdate(
|
||||
"page1",
|
||||
res.nodeId,
|
||||
UPDATED_MODEL,
|
||||
got.meta.hash,
|
||||
);
|
||||
assert.equal(upd.success, true);
|
||||
assert.equal(upd.nodeId, res.nodeId);
|
||||
const updated = findDrawio(upClient.calls.mutations[0].doc)[0];
|
||||
assert.equal(
|
||||
decodeDrawioSvg(upClient.calls.uploads[0].svg),
|
||||
normalizeXml(UPDATED_MODEL),
|
||||
);
|
||||
assert.equal(updated.attrs.width, 340);
|
||||
});
|
||||
|
||||
// --- error paths: the LLM must get a clean error, not a crash --------------
|
||||
|
||||
test("drawio_get: a bad node ref -> clean 'no node found' error", async () => {
|
||||
// Page has one paragraph; the requested ref resolves to nothing.
|
||||
const pageDoc = {
|
||||
type: "doc",
|
||||
content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }],
|
||||
};
|
||||
const { client } = makeClient({ pageDoc, attachmentSvg: svgFor(MODEL) });
|
||||
await assert.rejects(
|
||||
() => client.drawioGet("page1", "does-not-exist", "xml"),
|
||||
/no node found for "does-not-exist"/,
|
||||
);
|
||||
});
|
||||
|
||||
test("drawio_get: a drawio node with no src -> clean 'has no src to read' error", async () => {
|
||||
const pageDoc = {
|
||||
type: "doc",
|
||||
content: [
|
||||
// A drawio node that carries no `src` (e.g. a half-written node).
|
||||
{ type: "drawio", attrs: { id: "d1", attachmentId: "att-1" } },
|
||||
],
|
||||
};
|
||||
const { client } = makeClient({ pageDoc, attachmentSvg: svgFor(MODEL) });
|
||||
await assert.rejects(
|
||||
() => client.drawioGet("page1", "d1", "xml"),
|
||||
/node "d1" on page page1 has no src to read/,
|
||||
);
|
||||
});
|
||||
|
||||
test("drawio_update: the resolved node is NOT a drawio node -> clean error, no upload", async () => {
|
||||
// "#0" resolves to a paragraph. The update must refuse cleanly rather than
|
||||
// crash or repoint the wrong node.
|
||||
const pageDoc = {
|
||||
type: "doc",
|
||||
content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }],
|
||||
};
|
||||
const { client, calls } = makeClient({ pageDoc, attachmentSvg: svgFor(MODEL) });
|
||||
await assert.rejects(
|
||||
() => client.drawioUpdate("page1", "#0", UPDATED_MODEL, "any-nonempty-hash"),
|
||||
/node "#0" on page page1 is a paragraph, not a drawio diagram/,
|
||||
);
|
||||
assert.equal(calls.uploads.length, 0, "no upload when the node is not a diagram");
|
||||
assert.equal(calls.mutations.length, 0, "no write when the node is not a diagram");
|
||||
});
|
||||
|
||||
test("drawio_create: anchor not found -> clean error that reports the orphan attachment", async () => {
|
||||
// The upload happens before the mutate transform; when the anchor cannot be
|
||||
// found the write is skipped and the (now unreferenced) attachment is named
|
||||
// in the error, exactly as the code documents.
|
||||
const pageDoc = {
|
||||
type: "doc",
|
||||
content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }],
|
||||
};
|
||||
const { client, calls } = makeClient({ pageDoc });
|
||||
await assert.rejects(
|
||||
() =>
|
||||
client.drawioCreate(
|
||||
"page1",
|
||||
{ position: "after", anchorNodeId: "nope" },
|
||||
MODEL,
|
||||
"T",
|
||||
),
|
||||
(err) =>
|
||||
/anchor not found/.test(err.message) &&
|
||||
/unreferenced orphan/.test(err.message) &&
|
||||
/att-1/.test(err.message),
|
||||
);
|
||||
// The orphan was uploaded (and reported), but no node was written.
|
||||
assert.equal(calls.uploads.length, 1, "attachment uploaded before the failed insert");
|
||||
const drawios = calls.mutations.length ? findDrawio(calls.mutations[0].doc) : [];
|
||||
assert.equal(drawios.length, 0, "no drawio node written when the anchor is missing");
|
||||
});
|
||||
|
||||
// --- Fix 2: update targets ONLY the resolved node --------------------------
|
||||
|
||||
test("drawio_update: repoints ONLY the addressed node, not siblings sharing an attachmentId", async () => {
|
||||
// A copied diagram: two drawio nodes share one attachmentId. Updating via the
|
||||
// "#0" handle must touch node #0 only, never the sibling copy.
|
||||
const shared = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "drawio",
|
||||
attrs: {
|
||||
src: "/api/files/shared/x.svg",
|
||||
attachmentId: "shared",
|
||||
width: 200,
|
||||
height: 120,
|
||||
},
|
||||
},
|
||||
{
|
||||
type: "drawio",
|
||||
attrs: {
|
||||
src: "/api/files/shared/x.svg",
|
||||
attachmentId: "shared",
|
||||
width: 200,
|
||||
height: 120,
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
const { client, calls } = makeClient({
|
||||
pageDoc: shared,
|
||||
attachmentSvg: svgFor(MODEL),
|
||||
});
|
||||
const res = await client.drawioUpdate(
|
||||
"page1",
|
||||
"#0",
|
||||
UPDATED_MODEL,
|
||||
mxHash(normalizeXml(MODEL)),
|
||||
);
|
||||
assert.equal(res.success, true);
|
||||
|
||||
const drawios = findDrawio(calls.mutations[0].doc);
|
||||
assert.equal(drawios.length, 2);
|
||||
// Node #0 repointed to the NEW attachment ("att-1" from the stub) and dims.
|
||||
assert.equal(drawios[0].attrs.attachmentId, "att-1");
|
||||
assert.equal(drawios[0].attrs.width, 340);
|
||||
assert.match(drawios[0].attrs.src, /^\/api\/files\/att-1\//);
|
||||
// Node #1 (the sibling copy) is untouched despite sharing the old attachmentId.
|
||||
assert.equal(drawios[1].attrs.attachmentId, "shared");
|
||||
assert.equal(drawios[1].attrs.width, 200);
|
||||
assert.equal(drawios[1].attrs.src, "/api/files/shared/x.svg");
|
||||
});
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
applyAnchorInDoc,
|
||||
countAnchorMatches,
|
||||
getAnchoredText,
|
||||
resolveAnchorSelection,
|
||||
} from "../../build/lib/comment-anchor.js";
|
||||
|
||||
const COMMENT_ID = "cmt-123";
|
||||
@@ -308,3 +309,70 @@ test("getAnchoredText returns null when the selection does not anchor", () => {
|
||||
const doc = paragraphDoc([{ type: "text", text: "hello world" }]);
|
||||
assert.equal(getAnchoredText(doc, "not present"), null);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #408 MARKDOWN-STRIP FALLBACK. A selection copied with inline markdown still
|
||||
// carries `**`/`` ` ``/`[t](u)` markers the plain document text lacks. When the
|
||||
// verbatim selection anchors nowhere, all four entry points retry with the
|
||||
// markdown stripped — consistently, so the suggestion-uniqueness gate stays
|
||||
// coherent — while what gets STORED remains the raw document substring.
|
||||
// ---------------------------------------------------------------------------
|
||||
test("a markdown-styled selection anchors against plain doc text via the strip fallback", () => {
|
||||
const doc = paragraphDoc([{ type: "text", text: "a bold word here" }]);
|
||||
// The agent quoted "**bold** word" from a styled view; the doc is plain text.
|
||||
const sel = "**bold** word";
|
||||
const resolved = resolveAnchorSelection(doc, sel);
|
||||
assert.equal(resolved.found, true, "strip fallback finds the anchor");
|
||||
assert.equal(resolved.normalized, true, "reports the soft-warning flag");
|
||||
assert.equal(canAnchorInDoc(doc, sel), true);
|
||||
assert.equal(countAnchorMatches(doc, sel), 1);
|
||||
|
||||
const ok = applyAnchorInDoc(doc, sel, COMMENT_ID);
|
||||
assert.equal(ok, true);
|
||||
const marked = doc.content[0].content.filter((p) => commentMark(p));
|
||||
assert.equal(marked.map((m) => m.text).join(""), "bold word",
|
||||
"the mark lands on the plain-text span");
|
||||
});
|
||||
|
||||
test("getAnchoredText stores the RAW doc substring even when matched via the strip fallback", () => {
|
||||
// Doc uses a smart apostrophe; the agent typed ASCII + markdown emphasis.
|
||||
const doc = paragraphDoc([{ type: "text", text: "it’s bold now" }]);
|
||||
const stored = getAnchoredText(doc, "it's **bold**");
|
||||
assert.equal(stored, "it’s bold",
|
||||
"stored selection is the raw document text, not the stripped/ASCII locator");
|
||||
});
|
||||
|
||||
test("the strip fallback does not flip a raw-unique selection to ambiguous", () => {
|
||||
// "config" appears twice, but the raw phrase "config value" appears once.
|
||||
const doc = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "the config value here" }] },
|
||||
{ type: "paragraph", content: [{ type: "text", text: "another config here" }] },
|
||||
],
|
||||
};
|
||||
// Raw phrase is unique -> exactly 1, and no strip happens (nothing to strip).
|
||||
assert.equal(countAnchorMatches(doc, "config value"), 1);
|
||||
assert.equal(resolveAnchorSelection(doc, "config value").normalized, false);
|
||||
});
|
||||
|
||||
test("EXACT WINS: a raw match short-circuits the strip fallback (count reflects raw)", () => {
|
||||
// A literal "**" run exists raw once; its stripped form would also appear.
|
||||
const doc = paragraphDoc([{ type: "text", text: "use **stars** and stars" }]);
|
||||
// Raw "**stars**" occurs once -> count 1 from the verbatim locator; the
|
||||
// fallback (which would find two "stars") never runs.
|
||||
assert.equal(countAnchorMatches(doc, "**stars**"), 1);
|
||||
assert.equal(resolveAnchorSelection(doc, "**stars**").normalized, false);
|
||||
});
|
||||
|
||||
test("a markdown selection whose stripped form is ambiguous is counted as ambiguous", () => {
|
||||
const doc = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "first config here" }] },
|
||||
{ type: "paragraph", content: [{ type: "text", text: "second config here" }] },
|
||||
],
|
||||
};
|
||||
// Verbatim "**config**" matches nothing; stripped "config" matches twice.
|
||||
assert.equal(countAnchorMatches(doc, "**config**"), 2);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
// Unit tests for the pure-TS schematic SVG preview (issue #423). Asserts that
|
||||
// the preview emits well-formed SVG covering each primitive (rect/ellipse/
|
||||
// rhombus/edge), resolves container-relative coordinates to absolute, and that
|
||||
// the full `.drawio.svg` wrapper parses.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { JSDOM } from "jsdom";
|
||||
import { renderDiagramShapes } from "../../build/lib/drawio-preview.js";
|
||||
import {
|
||||
parseCells,
|
||||
computeBBox,
|
||||
buildDrawioSvg,
|
||||
normalizeXml,
|
||||
} from "../../build/lib/drawio-xml.js";
|
||||
|
||||
const { window } = new JSDOM("");
|
||||
function parseSvg(svg) {
|
||||
const doc = new window.DOMParser().parseFromString(svg, "application/xml");
|
||||
const err = doc.getElementsByTagName("parsererror");
|
||||
assert.equal(err.length, 0, `SVG did not parse: ${err[0]?.textContent}`);
|
||||
return doc;
|
||||
}
|
||||
|
||||
const MODEL =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" value="Box & Co" style="rounded=1;fillColor=#dae8fc;strokeColor=#6c8ebf;" vertex="1" parent="1">' +
|
||||
'<mxGeometry x="40" y="40" width="120" height="60" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="3" value="Circle" style="ellipse;fillColor=#d5e8d4;" vertex="1" parent="1">' +
|
||||
'<mxGeometry x="240" y="40" width="80" height="80" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="5" value="Dec" style="rhombus;" vertex="1" parent="1">' +
|
||||
'<mxGeometry x="40" y="160" width="100" height="60" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="4" value="link" edge="1" parent="1" source="2" target="3"><mxGeometry relative="1" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
|
||||
test("renderDiagramShapes emits rect, ellipse, polygon and a line", () => {
|
||||
const cells = parseCells(MODEL);
|
||||
const inner = renderDiagramShapes(cells, computeBBox(cells));
|
||||
assert.ok(inner.includes("<rect"), "has a rect");
|
||||
assert.ok(inner.includes("<ellipse"), "has an ellipse");
|
||||
assert.ok(inner.includes("<polygon"), "has a polygon (rhombus)");
|
||||
assert.ok(inner.includes("<line"), "has an edge line");
|
||||
// Labels are HTML-escaped.
|
||||
assert.ok(inner.includes("Box & Co"));
|
||||
});
|
||||
|
||||
test("the full .drawio.svg wrapper parses as valid XML", () => {
|
||||
const cells = parseCells(MODEL);
|
||||
const bbox = computeBBox(cells);
|
||||
const inner = renderDiagramShapes(cells, bbox);
|
||||
const svg = buildDrawioSvg(normalizeXml(MODEL), inner, bbox);
|
||||
const doc = parseSvg(svg);
|
||||
assert.equal(doc.documentElement.tagName, "svg");
|
||||
assert.ok(doc.documentElement.getAttribute("content"), "carries content=");
|
||||
// The visible children exist.
|
||||
assert.ok(doc.getElementsByTagName("rect").length >= 1);
|
||||
});
|
||||
|
||||
test("container children resolve to absolute coordinates", () => {
|
||||
// A group at (100,100) with a child rect at relative (10,10,20,20) -> abs 110,110.
|
||||
const model =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="g" style="group;" vertex="1" parent="1"><mxGeometry x="100" y="100" width="200" height="200" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="c" value="in" vertex="1" parent="g"><mxGeometry x="10" y="10" width="20" height="20" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
const cells = parseCells(model);
|
||||
const inner = renderDiagramShapes(cells, computeBBox(cells));
|
||||
// The child rect must be placed at absolute x=110,y=110.
|
||||
assert.ok(/<rect x="110" y="110"/.test(inner), `expected abs child rect, got: ${inner}`);
|
||||
});
|
||||
|
||||
test("unknown stencil (shape=mxgraph.*) degrades to a labeled rectangle", () => {
|
||||
const model =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" value="AWS" style="shape=mxgraph.aws4.lambda;" vertex="1" parent="1">' +
|
||||
'<mxGeometry x="0" y="0" width="60" height="60" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
const cells = parseCells(model);
|
||||
const inner = renderDiagramShapes(cells, computeBBox(cells));
|
||||
assert.ok(inner.includes("<rect"), "unknown stencil -> rect");
|
||||
assert.ok(inner.includes(">AWS<"), "keeps the label");
|
||||
});
|
||||
@@ -0,0 +1,360 @@
|
||||
// Unit tests for the drawio-xml module (issue #423): the linter (a positive
|
||||
// baseline + a negative case per rule, each asserting rule + cellId), the
|
||||
// decode chain (plain nested XML AND draw.io's compressed <diagram> via pako),
|
||||
// encode/round-trip byte-stability, hash stability, style parsing and the
|
||||
// bounding box.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import pako from "pako";
|
||||
import {
|
||||
parseStyle,
|
||||
lintModel,
|
||||
prepareModel,
|
||||
normalizeInput,
|
||||
normalizeXml,
|
||||
mxHash,
|
||||
computeBBox,
|
||||
parseCells,
|
||||
decodeDrawioSvg,
|
||||
decodeDrawioFileToModel,
|
||||
buildDrawioSvg,
|
||||
encodeDrawioFile,
|
||||
countUserCells,
|
||||
DrawioLintError,
|
||||
inflateDiagramPayload,
|
||||
MAX_INFLATED_DIAGRAM_BYTES,
|
||||
} from "../../build/lib/drawio-xml.js";
|
||||
|
||||
// A well-formed model with one vertex and a valid edge to it.
|
||||
const VALID_MODEL =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" value="Hello" style="rounded=1;whiteSpace=wrap;html=1;" vertex="1" parent="1">' +
|
||||
'<mxGeometry x="100" y="100" width="120" height="60" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="3" value="Two" style="ellipse;" vertex="1" parent="1">' +
|
||||
'<mxGeometry x="300" y="100" width="80" height="80" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="4" edge="1" parent="1" source="2" target="3">' +
|
||||
'<mxGeometry relative="1" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
|
||||
function issuesOf(fn) {
|
||||
try {
|
||||
fn();
|
||||
return null;
|
||||
} catch (e) {
|
||||
assert.ok(e instanceof DrawioLintError, `expected DrawioLintError, got ${e}`);
|
||||
return e.issues;
|
||||
}
|
||||
}
|
||||
const hasRule = (issues, rule, cellId) =>
|
||||
issues.some(
|
||||
(i) => i.rule === rule && (cellId === undefined || i.cellId === cellId),
|
||||
);
|
||||
|
||||
// --- style parsing ---------------------------------------------------------
|
||||
|
||||
test("parseStyle: base stylename + key=value pairs", () => {
|
||||
const r = parseStyle("ellipse;fillColor=#ff0000;whiteSpace=wrap;");
|
||||
assert.equal(r.baseStyle, "ellipse");
|
||||
assert.equal(r.map.fillColor, "#ff0000");
|
||||
assert.equal(r.map.whiteSpace, "wrap");
|
||||
assert.equal(r.badSegment, undefined);
|
||||
});
|
||||
|
||||
test("parseStyle: flags a segment with two '='", () => {
|
||||
const r = parseStyle("a=b=c;");
|
||||
assert.equal(r.badSegment, "a=b=c");
|
||||
});
|
||||
|
||||
test("parseStyle: a second bare token is malformed", () => {
|
||||
const r = parseStyle("rounded=1;bareword");
|
||||
assert.equal(r.badSegment, "bareword");
|
||||
});
|
||||
|
||||
// --- linter: positive baseline ---------------------------------------------
|
||||
|
||||
test("lintModel: the canonical valid model passes", () => {
|
||||
const { cells } = lintModel(VALID_MODEL);
|
||||
assert.equal(cells.length, 5);
|
||||
});
|
||||
|
||||
// --- linter: one negative case per rule ------------------------------------
|
||||
|
||||
test("rule well-formed-xml: malformed XML", () => {
|
||||
const issues = issuesOf(() => lintModel("<mxGraphModel><root><mxCell id=\"0\"></root>"));
|
||||
assert.ok(hasRule(issues, "well-formed-xml"));
|
||||
assert.ok(issues[0].position, "carries a line:col position");
|
||||
});
|
||||
|
||||
test("rule structure: root is not mxGraphModel", () => {
|
||||
const issues = issuesOf(() => lintModel("<foo><root/></foo>"));
|
||||
assert.ok(hasRule(issues, "structure"));
|
||||
});
|
||||
|
||||
test("rule sentinel-cells: missing id=0 / id=1", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
const issues = issuesOf(() => lintModel(m));
|
||||
assert.ok(hasRule(issues, "sentinel-cells", "0"));
|
||||
assert.ok(hasRule(issues, "sentinel-cells", "1"));
|
||||
});
|
||||
|
||||
test("rule duplicate-id: two cells share an id", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
const issues = issuesOf(() => lintModel(m));
|
||||
assert.ok(hasRule(issues, "duplicate-id", "2"));
|
||||
});
|
||||
|
||||
test("rule vertex-edge-exclusive: cell is both vertex and edge", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" vertex="1" edge="1" parent="1"><mxGeometry as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
const issues = issuesOf(() => lintModel(m));
|
||||
assert.ok(hasRule(issues, "vertex-edge-exclusive", "2"));
|
||||
});
|
||||
|
||||
test("rule edge-geometry: self-closed edge without child mxGeometry", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="3" vertex="1" parent="1"><mxGeometry x="30" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="4" edge="1" parent="1" source="2" target="3"/>' +
|
||||
'</root></mxGraphModel>';
|
||||
const issues = issuesOf(() => lintModel(m));
|
||||
assert.ok(hasRule(issues, "edge-geometry", "4"));
|
||||
});
|
||||
|
||||
test("rule edge-endpoint: source/target does not resolve", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'<mxCell id="4" edge="1" parent="1" source="2" target="99"><mxGeometry relative="1" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
const issues = issuesOf(() => lintModel(m));
|
||||
assert.ok(hasRule(issues, "edge-endpoint", "4"));
|
||||
});
|
||||
|
||||
test("rule parent-exists: parent points at a missing id", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" vertex="1" parent="42"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
const issues = issuesOf(() => lintModel(m));
|
||||
assert.ok(hasRule(issues, "parent-exists", "2"));
|
||||
});
|
||||
|
||||
test("rule no-comments: XML comment present", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<!-- a comment --><mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'</root></mxGraphModel>';
|
||||
const issues = issuesOf(() => lintModel(m));
|
||||
assert.ok(hasRule(issues, "no-comments"));
|
||||
});
|
||||
|
||||
test("rule style-format: malformed style segment (cellId reported)", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" style="rounded=1;a=b=c;" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
const issues = issuesOf(() => lintModel(m));
|
||||
assert.ok(hasRule(issues, "style-format", "2"));
|
||||
});
|
||||
|
||||
test("rule value-newline: literal newline in a value (cellId reported)", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" value="line1\nline2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
const issues = issuesOf(() => lintModel(m));
|
||||
assert.ok(hasRule(issues, "value-newline", "2"));
|
||||
});
|
||||
|
||||
test("rule value-escaping: unescaped ampersand in a value (cellId reported)", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" value="A & B" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
const issues = issuesOf(() => lintModel(m));
|
||||
assert.ok(hasRule(issues, "value-escaping", "2"));
|
||||
});
|
||||
|
||||
test("rule reserved id: escaped entity value passes (no false positive)", () => {
|
||||
const m =
|
||||
'<mxGraphModel><root>' +
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" value="A & B <ok>" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>' +
|
||||
'</root></mxGraphModel>';
|
||||
assert.doesNotThrow(() => lintModel(m));
|
||||
});
|
||||
|
||||
// --- input normalization ---------------------------------------------------
|
||||
|
||||
test("normalizeInput: a list of <mxCell> is wrapped and sentinels added", () => {
|
||||
const frag =
|
||||
'<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>';
|
||||
const model = normalizeInput(frag);
|
||||
assert.ok(model.startsWith("<mxGraphModel"));
|
||||
assert.ok(model.includes('<mxCell id="0"/>'));
|
||||
assert.ok(model.includes('<mxCell id="1" parent="0"/>'));
|
||||
// And it lints clean.
|
||||
assert.doesNotThrow(() => lintModel(model));
|
||||
});
|
||||
|
||||
test("normalizeInput: an existing sentinel is not duplicated", () => {
|
||||
const frag =
|
||||
'<mxCell id="0"/><mxCell id="1" parent="0"/>' +
|
||||
'<mxCell id="2" vertex="1" parent="1"><mxGeometry x="0" y="0" width="10" height="10" as="geometry"/></mxCell>';
|
||||
const model = normalizeInput(frag);
|
||||
const count0 = (model.match(/id="0"/g) || []).length;
|
||||
assert.equal(count0, 1);
|
||||
});
|
||||
|
||||
test("prepareModel: returns bbox, cellCount, hash and lints", () => {
|
||||
const p = prepareModel(VALID_MODEL);
|
||||
assert.equal(p.cellCount, 3); // 2, 3, 4 (sentinels excluded)
|
||||
assert.ok(p.bbox.width > 0 && p.bbox.height > 0);
|
||||
assert.equal(p.hash, mxHash(normalizeXml(VALID_MODEL)));
|
||||
});
|
||||
|
||||
// --- decode chain: plain -----------------------------------------------------
|
||||
|
||||
test("decode chain (plain): buildDrawioSvg -> decodeDrawioSvg round-trips byte-stable", () => {
|
||||
const model = normalizeXml(VALID_MODEL);
|
||||
const svg = buildDrawioSvg(model, "<g/>", { width: 400, height: 200 });
|
||||
const decoded = decodeDrawioSvg(svg);
|
||||
assert.equal(decoded, model);
|
||||
});
|
||||
|
||||
test("decode chain: entity-encoded content= (draw.io export style) is read directly", () => {
|
||||
const model = normalizeXml(VALID_MODEL);
|
||||
const file = encodeDrawioFile(model, "Page-1");
|
||||
const escaped = file
|
||||
.replace(/&/g, "&")
|
||||
.replace(/</g, "<")
|
||||
.replace(/>/g, ">")
|
||||
.replace(/"/g, """);
|
||||
const svg = `<svg xmlns="http://www.w3.org/2000/svg" content="${escaped}"></svg>`;
|
||||
const decoded = decodeDrawioSvg(svg);
|
||||
assert.equal(decoded, model);
|
||||
});
|
||||
|
||||
// --- decode chain: compressed (pako) ---------------------------------------
|
||||
|
||||
test("decode chain (compressed pako): human-saved <diagram> payload decodes losslessly", () => {
|
||||
const model = normalizeXml(VALID_MODEL);
|
||||
// Reproduce draw.io's compression: encodeURIComponent -> raw deflate -> base64.
|
||||
const compressed = Buffer.from(
|
||||
pako.deflateRaw(encodeURIComponent(model)),
|
||||
).toString("base64");
|
||||
const file = `<mxfile host="Electron"><diagram id="abc" name="Page-1">${compressed}</diagram></mxfile>`;
|
||||
// Docmost stores the file base64 in content=.
|
||||
const contentB64 = Buffer.from(file, "utf-8").toString("base64");
|
||||
const svg = `<svg xmlns="http://www.w3.org/2000/svg" content="${contentB64}"><image href="x"/></svg>`;
|
||||
const decoded = decodeDrawioSvg(svg);
|
||||
assert.equal(decoded, model);
|
||||
});
|
||||
|
||||
test("decodeDrawioFileToModel: bare mxGraphModel file returns the model substring", () => {
|
||||
const model = normalizeXml(VALID_MODEL);
|
||||
assert.equal(decodeDrawioFileToModel(model), model);
|
||||
});
|
||||
|
||||
// --- hash stability --------------------------------------------------------
|
||||
|
||||
test("mxHash: stable across inter-tag whitespace, sensitive to content", () => {
|
||||
const a = VALID_MODEL;
|
||||
const b = VALID_MODEL.replace(/></g, ">\n <"); // reformat only
|
||||
assert.equal(mxHash(a), mxHash(b));
|
||||
const c = VALID_MODEL.replace('value="Hello"', 'value="Changed"');
|
||||
assert.notEqual(mxHash(a), mxHash(c));
|
||||
});
|
||||
|
||||
// --- bounding box + cell count ---------------------------------------------
|
||||
|
||||
test("computeBBox + countUserCells", () => {
|
||||
const cells = parseCells(VALID_MODEL);
|
||||
const bbox = computeBBox(cells);
|
||||
// Vertex 3 spans to x=380,y=180; plus the 20px margin.
|
||||
assert.equal(bbox.width, 400);
|
||||
assert.equal(bbox.height, 200);
|
||||
assert.equal(countUserCells(VALID_MODEL), 3);
|
||||
});
|
||||
|
||||
// --- decompression-bomb guard (Fix 3) --------------------------------------
|
||||
|
||||
test("inflateDiagramPayload: a small legitimate payload inflates fine", () => {
|
||||
const xml = normalizeXml(VALID_MODEL);
|
||||
const base64 = Buffer.from(
|
||||
pako.deflateRaw(encodeURIComponent(xml)),
|
||||
).toString("base64");
|
||||
assert.equal(inflateDiagramPayload(base64), xml);
|
||||
});
|
||||
|
||||
test("inflateDiagramPayload: rejects an over-cap decompression bomb", () => {
|
||||
// A tiny compressed payload that inflates to just over the cap. Highly
|
||||
// compressible (all one byte) -> the base64 is small, but the inflated output
|
||||
// exceeds MAX_INFLATED_DIAGRAM_BYTES and must be refused before it is fully
|
||||
// materialised.
|
||||
const bombSize = MAX_INFLATED_DIAGRAM_BYTES + 1024;
|
||||
const base64 = Buffer.from(
|
||||
pako.deflateRaw(Buffer.alloc(bombSize, 0x41 /* 'A' */)),
|
||||
).toString("base64");
|
||||
assert.ok(
|
||||
base64.length < 1024 * 1024,
|
||||
"the compressed bomb is tiny relative to its inflated size",
|
||||
);
|
||||
assert.throws(
|
||||
() => inflateDiagramPayload(base64),
|
||||
/decompression bomb/,
|
||||
);
|
||||
});
|
||||
|
||||
test("encode/build: a title with < > \" & round-trips without corrupting the SVG", async () => {
|
||||
// A user-supplied title full of XML metacharacters must be escaped so the
|
||||
// inner <mxfile> stays well-formed and the outer content="..." attribute is
|
||||
// never broken out of. Prove it survives the encode -> build -> decode chain.
|
||||
const title = 'A < B > C " D & E';
|
||||
const model = normalizeXml(VALID_MODEL);
|
||||
const svg = buildDrawioSvg(model, "<g/>", { width: 200, height: 120 }, title);
|
||||
|
||||
// The outer content="..." attribute must not be broken by the title: the raw
|
||||
// title metacharacters never appear literally in the SVG markup (they are
|
||||
// base64-encoded inside content=, and escaped inside the file XML).
|
||||
const contentMatch = /content="([^"]*)"/.exec(svg);
|
||||
assert.ok(contentMatch, "SVG has a single well-formed content= attribute");
|
||||
|
||||
// The diagram model still decodes losslessly despite the exotic title.
|
||||
assert.equal(decodeDrawioSvg(svg), model);
|
||||
|
||||
// The file XML is well-formed: the title lives in name="..." as escaped
|
||||
// entities, so unescaping recovers the original title byte-for-byte.
|
||||
const fileXml = Buffer.from(contentMatch[1], "base64").toString("utf-8");
|
||||
const nameMatch = /<diagram id="[^"]*" name="([^"]*)">/.exec(fileXml);
|
||||
assert.ok(nameMatch, "the diagram name attribute is intact and quote-safe");
|
||||
const decodedTitle = nameMatch[1]
|
||||
.replace(/</g, "<")
|
||||
.replace(/>/g, ">")
|
||||
.replace(/"/g, '"')
|
||||
.replace(/&/g, "&");
|
||||
assert.equal(decodedTitle, title);
|
||||
|
||||
// encodeDrawioFile alone produces the same escaped, well-formed envelope.
|
||||
const file = encodeDrawioFile(model, title);
|
||||
assert.match(file, /name="A < B > C " D & E">/);
|
||||
});
|
||||
@@ -0,0 +1,62 @@
|
||||
diff --git a/dist/hocuspocus-server.cjs b/dist/hocuspocus-server.cjs
|
||||
index b24ff6d091c32f733089eeaa47b03f7b37cf5964..f003af304fc751b7edc1aee17f3651282d70666a 100644
|
||||
--- a/dist/hocuspocus-server.cjs
|
||||
+++ b/dist/hocuspocus-server.cjs
|
||||
@@ -2426,6 +2426,26 @@ class Hocuspocus {
|
||||
* Create a new document by the given request
|
||||
*/
|
||||
async createDocument(documentName, request, socketId, connection, context) {
|
||||
+ // PATCH(gitmost #401): close the connect-vs-unload race. When the last
|
||||
+ // client disconnects, storeDocumentHooks' finally schedules an unload;
|
||||
+ // unloadDocument runs its beforeUnloadDocument hooks asynchronously and
|
||||
+ // records an in-flight promise in `unloadingDocuments`. A NEW connection
|
||||
+ // arriving in that window would otherwise fall straight through to the
|
||||
+ // `documents`/`loadingDocuments` checks and could either reuse a doc that
|
||||
+ // is about to be destroyed, or start loading a fresh doc concurrently
|
||||
+ // with the destroy. Awaiting the in-flight unload first makes the decision
|
||||
+ // deterministic: once it settles, either the doc was fully unloaded
|
||||
+ // (removed from `documents`, so we do a clean fresh load below) or the
|
||||
+ // unload aborted because work/connections reappeared (the healthy doc is
|
||||
+ // still in `documents`, so we reuse it). Either way the new connection can
|
||||
+ // never hand-shake onto an orphaned, about-to-be-destroyed Document.
|
||||
+ const existingUnloadingDoc = this.unloadingDocuments.get(documentName);
|
||||
+ if (existingUnloadingDoc) {
|
||||
+ // Wait for the in-flight unload to settle so we never hand-shake onto a dying
|
||||
+ // Document. Swallow a rejected unload — fall through to a fresh load, matching
|
||||
+ // pre-patch behavior (the doc is already removed from `documents` by then).
|
||||
+ try { await existingUnloadingDoc; } catch { /* unload rejected — fresh load */ }
|
||||
+ }
|
||||
const existingLoadingDoc = this.loadingDocuments.get(documentName);
|
||||
if (existingLoadingDoc) {
|
||||
return existingLoadingDoc;
|
||||
diff --git a/dist/hocuspocus-server.esm.js b/dist/hocuspocus-server.esm.js
|
||||
index 1f4dd80244e899128e2c4e5dad8eab7cfc1cbad6..8c2411747bba27fb9486e1df81678a14e41e884e 100644
|
||||
--- a/dist/hocuspocus-server.esm.js
|
||||
+++ b/dist/hocuspocus-server.esm.js
|
||||
@@ -2406,6 +2406,26 @@ class Hocuspocus {
|
||||
* Create a new document by the given request
|
||||
*/
|
||||
async createDocument(documentName, request, socketId, connection, context) {
|
||||
+ // PATCH(gitmost #401): close the connect-vs-unload race. When the last
|
||||
+ // client disconnects, storeDocumentHooks' finally schedules an unload;
|
||||
+ // unloadDocument runs its beforeUnloadDocument hooks asynchronously and
|
||||
+ // records an in-flight promise in `unloadingDocuments`. A NEW connection
|
||||
+ // arriving in that window would otherwise fall straight through to the
|
||||
+ // `documents`/`loadingDocuments` checks and could either reuse a doc that
|
||||
+ // is about to be destroyed, or start loading a fresh doc concurrently
|
||||
+ // with the destroy. Awaiting the in-flight unload first makes the decision
|
||||
+ // deterministic: once it settles, either the doc was fully unloaded
|
||||
+ // (removed from `documents`, so we do a clean fresh load below) or the
|
||||
+ // unload aborted because work/connections reappeared (the healthy doc is
|
||||
+ // still in `documents`, so we reuse it). Either way the new connection can
|
||||
+ // never hand-shake onto an orphaned, about-to-be-destroyed Document.
|
||||
+ const existingUnloadingDoc = this.unloadingDocuments.get(documentName);
|
||||
+ if (existingUnloadingDoc) {
|
||||
+ // Wait for the in-flight unload to settle so we never hand-shake onto a dying
|
||||
+ // Document. Swallow a rejected unload — fall through to a fresh load, matching
|
||||
+ // pre-patch behavior (the doc is already removed from `documents` by then).
|
||||
+ try { await existingUnloadingDoc; } catch { /* unload rejected — fresh load */ }
|
||||
+ }
|
||||
const existingLoadingDoc = this.loadingDocuments.get(documentName);
|
||||
if (existingLoadingDoc) {
|
||||
return existingLoadingDoc;
|
||||
Generated
+8
-2
@@ -44,6 +44,9 @@ overrides:
|
||||
ip-address: 10.1.1
|
||||
|
||||
patchedDependencies:
|
||||
'@hocuspocus/server@3.4.4':
|
||||
hash: d8dc66e5ec3b9d23a876b979f493b6aa901fd2d965be54729495da5136296a42
|
||||
path: patches/@hocuspocus__server@3.4.4.patch
|
||||
ai@6.0.134:
|
||||
hash: f60bfc3357e01e1f3978c6c40fdd65aeb33fefaad7179cde8676465b6c5ff4d9
|
||||
path: patches/ai@6.0.134.patch
|
||||
@@ -75,7 +78,7 @@ importers:
|
||||
version: 3.4.4(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))
|
||||
'@hocuspocus/server':
|
||||
specifier: 3.4.4
|
||||
version: 3.4.4(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))
|
||||
version: 3.4.4(patch_hash=d8dc66e5ec3b9d23a876b979f493b6aa901fd2d965be54729495da5136296a42)(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))
|
||||
'@hocuspocus/transformer':
|
||||
specifier: 3.4.4
|
||||
version: 3.4.4(@tiptap/core@3.20.4(@tiptap/pm@3.20.4))(@tiptap/pm@3.20.4)(y-prosemirror@1.3.7(prosemirror-model@1.25.1)(prosemirror-state@1.4.3)(prosemirror-view@1.40.0)(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))
|
||||
@@ -1041,6 +1044,9 @@ importers:
|
||||
marked:
|
||||
specifier: ^17.0.1
|
||||
version: 17.0.5
|
||||
pako:
|
||||
specifier: ^2.0.3
|
||||
version: 2.0.3
|
||||
re2:
|
||||
specifier: ^1.21.0
|
||||
version: 1.25.0
|
||||
@@ -13055,7 +13061,7 @@ snapshots:
|
||||
- bufferutil
|
||||
- utf-8-validate
|
||||
|
||||
'@hocuspocus/server@3.4.4(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))':
|
||||
'@hocuspocus/server@3.4.4(patch_hash=d8dc66e5ec3b9d23a876b979f493b6aa901fd2d965be54729495da5136296a42)(y-protocols@1.0.6(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810)))(yjs@13.6.30(patch_hash=1ceeb66dba1f86545c98a3ff7f5152aff9b35caf409091cef9caedb5e65c8810))':
|
||||
dependencies:
|
||||
'@hocuspocus/common': 3.4.4
|
||||
async-lock: 1.4.1
|
||||
|
||||
Reference in New Issue
Block a user