Compare commits
57 Commits
test/244-p
...
test/244-p
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
42a1fa1d3a | ||
|
|
ef27b6d440 | ||
|
|
96b9ec11d6 | ||
|
|
f8d26420eb | ||
|
|
5c1187b864 | ||
|
|
14f83abe78 | ||
| 22ea387495 | |||
| b56a1629d2 | |||
| 7e6dd457a4 | |||
| ad08458ac4 | |||
|
|
9bbac29bc5 | ||
| 42f3a328c2 | |||
| a8a7fad850 | |||
|
|
f9d8a6ede1 | ||
|
|
3c7b69d6d4 | ||
| d38a39e3e5 | |||
|
|
0724d8d362 | ||
| 116a231691 | |||
|
|
188c5f506c | ||
| e5a0f2d887 | |||
|
|
97eef22bc3 | ||
|
|
aa14ad6698 | ||
|
|
1e5994573f | ||
|
|
d0eae69086 | ||
|
|
91f24fc062 | ||
|
|
888deba891 | ||
|
|
82b042209e | ||
|
|
a0f4c86a74 | ||
|
|
cce539e8e2 | ||
|
|
8274720281 | ||
|
|
3fdb1e05a4 | ||
|
|
57308bc3f3 | ||
|
|
4c7b671950 | ||
|
|
90a3fa012d | ||
|
|
bdc033e689 | ||
|
|
1ddb386214 | ||
|
|
43af3dd5f1 | ||
|
|
b02101b58a | ||
|
|
932bfce1d9 | ||
|
|
04fda0c0b2 | ||
|
|
4131deaabb | ||
|
|
5308f2fb65 | ||
|
|
78cc019492 | ||
|
|
85b38d6946 | ||
|
|
d39b7ae67c | ||
|
|
c124fb1f2c | ||
|
|
d3ebae48cf | ||
|
|
607aed5997 | ||
|
|
5b88e3dddf | ||
|
|
d0ca127d83 | ||
|
|
78953cf775 | ||
|
|
bf09eec4e1 | ||
|
|
dc14a9a540 | ||
|
|
2aa482f62d | ||
|
|
95d07d8d6f | ||
|
|
630939e8f3 | ||
|
|
72bb03918d |
14
.github/workflows/develop.yml
vendored
14
.github/workflows/develop.yml
vendored
@@ -75,7 +75,9 @@ jobs:
|
||||
APP_URL: http://localhost:3000
|
||||
services:
|
||||
postgres:
|
||||
image: pgvector/pgvector:pg18
|
||||
# via mirror.gcr.io (Docker Hub pull-through cache; avoids Hub anonymous
|
||||
# pull rate-limit that randomly fails on shared GitHub runner IPs).
|
||||
image: mirror.gcr.io/pgvector/pgvector:pg18
|
||||
env:
|
||||
POSTGRES_DB: docmost
|
||||
POSTGRES_USER: docmost
|
||||
@@ -88,7 +90,8 @@ jobs:
|
||||
--health-timeout 5s
|
||||
--health-retries 20
|
||||
redis:
|
||||
image: redis:7
|
||||
# via mirror.gcr.io (see postgres note above).
|
||||
image: mirror.gcr.io/library/redis:7
|
||||
ports:
|
||||
- 6379:6379
|
||||
options: >-
|
||||
@@ -135,7 +138,9 @@ jobs:
|
||||
NODE_ENV: production
|
||||
services:
|
||||
postgres:
|
||||
image: pgvector/pgvector:pg18
|
||||
# via mirror.gcr.io (Docker Hub pull-through cache; avoids Hub anonymous
|
||||
# pull rate-limit that randomly fails on shared GitHub runner IPs).
|
||||
image: mirror.gcr.io/pgvector/pgvector:pg18
|
||||
env:
|
||||
POSTGRES_DB: docmost
|
||||
POSTGRES_USER: docmost
|
||||
@@ -148,7 +153,8 @@ jobs:
|
||||
--health-timeout 5s
|
||||
--health-retries 20
|
||||
redis:
|
||||
image: redis:7
|
||||
# via mirror.gcr.io (see postgres note above).
|
||||
image: mirror.gcr.io/library/redis:7
|
||||
ports:
|
||||
- 6379:6379
|
||||
options: >-
|
||||
|
||||
7
.github/workflows/test.yml
vendored
7
.github/workflows/test.yml
vendored
@@ -27,7 +27,9 @@ jobs:
|
||||
# TEST_*_URL overrides are needed.
|
||||
services:
|
||||
postgres:
|
||||
image: pgvector/pgvector:pg18
|
||||
# via mirror.gcr.io (Docker Hub pull-through cache; avoids Hub anonymous
|
||||
# pull rate-limit that randomly fails on shared GitHub runner IPs).
|
||||
image: mirror.gcr.io/pgvector/pgvector:pg18
|
||||
env:
|
||||
POSTGRES_USER: docmost
|
||||
POSTGRES_PASSWORD: docmost_dev_pw
|
||||
@@ -40,7 +42,8 @@ jobs:
|
||||
--health-timeout 5s
|
||||
--health-retries 5
|
||||
redis:
|
||||
image: redis:7
|
||||
# via mirror.gcr.io (see postgres note above).
|
||||
image: mirror.gcr.io/library/redis:7
|
||||
ports:
|
||||
- 6379:6379
|
||||
options: >-
|
||||
|
||||
18
CHANGELOG.md
18
CHANGELOG.md
@@ -12,6 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Added
|
||||
|
||||
- **Editable captions for images.** Images gain an optional caption shown
|
||||
below them, edited inline from the image bubble menu and stored as a `caption` attribute. Captions round-trip
|
||||
losslessly through markdown as a `data-caption` attribute on the image, so
|
||||
they survive export/import unchanged. (#221)
|
||||
|
||||
- **Quick-create regular and temporary notes from the Home and Space screens.**
|
||||
The Home screen now shows a second action next to "New note" that creates a
|
||||
*temporary* note (one that auto-moves to Trash after the workspace lifetime),
|
||||
@@ -67,6 +72,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
`nosniff` + restrictive CSP + attachment disposition for non-image mimes) and
|
||||
are RAM-only, bound to the instance that created them. Tunable via five
|
||||
`SANDBOX_*` env vars (see `.env.example`). (#243)
|
||||
- **Inline spoiler mark — hide text behind click-to-reveal blur.** Selected text
|
||||
can be marked as a spoiler from a new bubble-menu toggle, or typed Discord-style
|
||||
with the `||text||` input rule; the rendered span blurs until clicked to reveal.
|
||||
The mark is preserved losslessly through Markdown export/import (as a raw
|
||||
`<span data-spoiler="true">…</span>`) and on public shares. (#259)
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -124,6 +134,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
"This address is in use. Saving will move it to this page." — and keeps Save
|
||||
enabled, so the existing reassign-confirm flow (`409 ALIAS_REASSIGN_REQUIRED` →
|
||||
"Move custom address?") is discoverable instead of reading as terminal. (#227)
|
||||
- **A non-empty page can no longer be silently lost to a momentarily-empty live
|
||||
document.** The server's persistence guard now refuses to overwrite non-empty
|
||||
persisted content with an empty live Y.Doc — a transient emptiness from a
|
||||
glitch, a bad merge, or an emptying transclusion no longer wipes the saved
|
||||
page. A *deliberate* clear still works: a select-all + Delete in the editor
|
||||
emits a single-use "intentional clear" signal that lets exactly that one empty
|
||||
write through the guard, so genuinely emptying a page is persisted while
|
||||
accidental empties are blocked. (#248, #251)
|
||||
|
||||
### Security
|
||||
|
||||
|
||||
@@ -286,6 +286,9 @@
|
||||
"Alt text": "Alt text",
|
||||
"Describe this for accessibility.": "Describe this for accessibility.",
|
||||
"Add a description": "Add a description",
|
||||
"Caption": "Caption",
|
||||
"Add a caption": "Add a caption",
|
||||
"Shown below the image.": "Shown below the image.",
|
||||
"Justify": "Justify",
|
||||
"Merge cells": "Merge cells",
|
||||
"Split cell": "Split cell",
|
||||
@@ -352,6 +355,7 @@
|
||||
"Underline": "Underline",
|
||||
"Strike": "Strike",
|
||||
"Code": "Code",
|
||||
"Spoiler": "Spoiler",
|
||||
"Comment": "Comment",
|
||||
"Text": "Text",
|
||||
"Heading 1": "Heading 1",
|
||||
|
||||
@@ -351,6 +351,7 @@
|
||||
"Underline": "Подчёркнутый",
|
||||
"Strike": "Перечёркнутый",
|
||||
"Code": "Код",
|
||||
"Spoiler": "Спойлер",
|
||||
"Comment": "Комментарий",
|
||||
"Text": "Текст",
|
||||
"Heading 1": "Заголовок 1",
|
||||
|
||||
@@ -0,0 +1,206 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { renderHook, act } from "@testing-library/react";
|
||||
|
||||
// Shared, hoisted test state the module mocks write into. `onSpeechEnd` is the
|
||||
// VAD callback the hook registers on MicVAD.new — capturing it lets us drive
|
||||
// "a speech segment ended" deterministically. `pending` collects the deferred
|
||||
// transcription promises so the test controls their resolution order, which is
|
||||
// the whole point: out-of-order HTTP responses must NOT scramble the emitted
|
||||
// text (the in-order emitter under test).
|
||||
const h = vi.hoisted(() => {
|
||||
return {
|
||||
onSpeechEnd: null as null | ((audio: Float32Array) => void),
|
||||
pending: [] as { resolve: (s: string) => void; reject: (e: unknown) => void }[],
|
||||
notify: null as null | ReturnType<typeof Object>,
|
||||
};
|
||||
});
|
||||
|
||||
// Lazy-imported VAD: capture the onSpeechEnd handler and hand back a no-op
|
||||
// instance (start/pause/destroy all resolve).
|
||||
vi.mock("@ricky0123/vad-web", () => ({
|
||||
MicVAD: {
|
||||
new: vi.fn(async (opts: { onSpeechEnd: (a: Float32Array) => void }) => {
|
||||
h.onSpeechEnd = opts.onSpeechEnd;
|
||||
return {
|
||||
start: vi.fn(async () => {}),
|
||||
pause: vi.fn(async () => {}),
|
||||
destroy: vi.fn(async () => {}),
|
||||
};
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
// Each transcribeAudio call returns a promise we resolve/reject by index.
|
||||
vi.mock("@/features/dictation/services/dictation-service", () => ({
|
||||
transcribeAudio: vi.fn(
|
||||
() =>
|
||||
new Promise<string>((resolve, reject) => {
|
||||
h.pending.push({ resolve, reject });
|
||||
}),
|
||||
),
|
||||
}));
|
||||
|
||||
// Avoid real WAV encoding; the segment payload is irrelevant to ordering.
|
||||
vi.mock("@/features/dictation/utils/encode-wav", () => ({
|
||||
encodeWavPcm16: vi.fn(() => new Blob()),
|
||||
}));
|
||||
|
||||
const notifyShow = vi.fn();
|
||||
vi.mock("@mantine/notifications", () => ({
|
||||
notifications: { show: (...args: unknown[]) => notifyShow(...args) },
|
||||
}));
|
||||
|
||||
vi.mock("react-i18next", () => ({
|
||||
useTranslation: () => ({ t: (s: string) => s }),
|
||||
}));
|
||||
|
||||
import { useStreamingDictation } from "./use-streaming-dictation";
|
||||
|
||||
// jsdom has no AudioContext; the hook constructs one and calls resume(). A
|
||||
// trivial stub is enough — the real audio path is irrelevant to ordering.
|
||||
class FakeAudioContext {
|
||||
state = "running";
|
||||
resume() {
|
||||
return Promise.resolve();
|
||||
}
|
||||
close() {
|
||||
this.state = "closed";
|
||||
return Promise.resolve();
|
||||
}
|
||||
}
|
||||
|
||||
async function startRecording(onText: (t: string) => void) {
|
||||
const hook = renderHook(() => useStreamingDictation({ onText }));
|
||||
await act(async () => {
|
||||
await hook.result.current.start();
|
||||
});
|
||||
// The VAD registered its onSpeechEnd and start() resolved into "recording".
|
||||
expect(h.onSpeechEnd).toBeTypeOf("function");
|
||||
expect(hook.result.current.status).toBe("recording");
|
||||
return hook;
|
||||
}
|
||||
|
||||
// Fire N ended speech segments (seq 0..N-1), each kicking off one transcription.
|
||||
async function emitSegments(n: number) {
|
||||
await act(async () => {
|
||||
for (let i = 0; i < n; i++) h.onSpeechEnd!(new Float32Array(8));
|
||||
});
|
||||
}
|
||||
|
||||
describe("useStreamingDictation — in-order segment emitter", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
h.onSpeechEnd = null;
|
||||
h.pending = [];
|
||||
notifyShow.mockClear();
|
||||
(window as unknown as { AudioContext: unknown }).AudioContext =
|
||||
FakeAudioContext;
|
||||
});
|
||||
|
||||
it("emits transcriptions in segment order even when responses resolve out of order", async () => {
|
||||
const emitted: string[] = [];
|
||||
await startRecording((t) => emitted.push(t));
|
||||
await emitSegments(3);
|
||||
expect(h.pending).toHaveLength(3);
|
||||
|
||||
// Resolve seq 1 FIRST: it must be buffered, not emitted, because seq 0 is
|
||||
// still outstanding (nextEmit == 0).
|
||||
await act(async () => {
|
||||
h.pending[1].resolve("second");
|
||||
});
|
||||
expect(emitted).toEqual([]);
|
||||
|
||||
// Resolve seq 0: this unblocks the buffer and flushes 0 then 1 in order.
|
||||
await act(async () => {
|
||||
h.pending[0].resolve("first");
|
||||
});
|
||||
expect(emitted).toEqual(["first", "second"]);
|
||||
|
||||
// seq 2 resolves last and flushes immediately (it is now next).
|
||||
await act(async () => {
|
||||
h.pending[2].resolve("third");
|
||||
});
|
||||
expect(emitted).toEqual(["first", "second", "third"]);
|
||||
});
|
||||
|
||||
it("trims whitespace and drops empty/whitespace-only transcriptions while still advancing", async () => {
|
||||
const emitted: string[] = [];
|
||||
await startRecording((t) => emitted.push(t));
|
||||
await emitSegments(3);
|
||||
|
||||
await act(async () => {
|
||||
h.pending[0].resolve(" hello "); // leading/trailing space trimmed
|
||||
h.pending[1].resolve(" "); // whitespace-only -> not emitted, but seq advances
|
||||
h.pending[2].resolve("world");
|
||||
});
|
||||
|
||||
expect(emitted).toEqual(["hello", "world"]);
|
||||
});
|
||||
|
||||
it("a failed segment shows one notification and is skipped so later segments still flush in order", async () => {
|
||||
const emitted: string[] = [];
|
||||
await startRecording((t) => emitted.push(t));
|
||||
await emitSegments(2);
|
||||
|
||||
// seq 0 fails: the user sees a notification and the emitter advances past it.
|
||||
await act(async () => {
|
||||
h.pending[0].reject({ message: "boom" });
|
||||
});
|
||||
expect(notifyShow).toHaveBeenCalledTimes(1);
|
||||
expect(emitted).toEqual([]);
|
||||
|
||||
// seq 1 still flushes (it is now next), proving one failure did not stall.
|
||||
await act(async () => {
|
||||
h.pending[1].resolve("survivor");
|
||||
});
|
||||
expect(emitted).toEqual(["survivor"]);
|
||||
});
|
||||
|
||||
it("an OUT-OF-ORDER failed segment is buffered as empty and skipped without stalling later text", async () => {
|
||||
const emitted: string[] = [];
|
||||
await startRecording((t) => emitted.push(t));
|
||||
await emitSegments(3);
|
||||
|
||||
// seq 1 (NOT next-to-emit) fails first: it takes the else branch — an empty
|
||||
// placeholder is buffered (resultsRef.set(seq, "")) so the emitter can later
|
||||
// skip it. One notification, nothing emitted yet (seq 0 still gates).
|
||||
await act(async () => {
|
||||
h.pending[1].reject({ message: "boom" });
|
||||
});
|
||||
expect(notifyShow).toHaveBeenCalledTimes(1);
|
||||
expect(emitted).toEqual([]);
|
||||
|
||||
// seq 0 flushes; the drain then reaches the buffered empty seq 1 and SKIPS
|
||||
// past it to seq 2.
|
||||
await act(async () => {
|
||||
h.pending[0].resolve("alpha");
|
||||
});
|
||||
expect(emitted).toEqual(["alpha"]);
|
||||
|
||||
// seq 2 emits — proving the empty placeholder let the emitter advance past
|
||||
// the failed seq 1. Without the else branch's placeholder the drain would
|
||||
// stall at the missing seq 1 and "gamma" would never flush.
|
||||
await act(async () => {
|
||||
h.pending[2].resolve("gamma");
|
||||
});
|
||||
expect(emitted).toEqual(["alpha", "gamma"]);
|
||||
});
|
||||
|
||||
it("ignores a transcription that resolves AFTER cancel() (stale epoch — no emit)", async () => {
|
||||
const emitted: string[] = [];
|
||||
const hook = await startRecording((t) => emitted.push(t));
|
||||
await emitSegments(1);
|
||||
|
||||
// Hard discard the session: the in-flight request is now stale.
|
||||
act(() => {
|
||||
hook.result.current.cancel();
|
||||
});
|
||||
expect(hook.result.current.status).toBe("idle");
|
||||
|
||||
// Its late resolution must be dropped (no emit into the new/empty session).
|
||||
await act(async () => {
|
||||
h.pending[0].resolve("late");
|
||||
});
|
||||
expect(emitted).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -9,6 +9,8 @@ import {
|
||||
IconStrikethrough,
|
||||
IconUnderline,
|
||||
IconMessage,
|
||||
IconEyeOff,
|
||||
IconClearFormatting,
|
||||
} from "@tabler/icons-react";
|
||||
import clsx from "clsx";
|
||||
import classes from "./bubble-menu.module.css";
|
||||
@@ -74,6 +76,7 @@ export const EditorBubbleMenu: FC<EditorBubbleMenuProps> = (props) => {
|
||||
isStrike: ctx.editor.isActive("strike"),
|
||||
isCode: ctx.editor.isActive("code"),
|
||||
isComment: ctx.editor.isActive("comment"),
|
||||
isSpoiler: ctx.editor.isActive("spoiler"),
|
||||
};
|
||||
},
|
||||
});
|
||||
@@ -109,6 +112,20 @@ export const EditorBubbleMenu: FC<EditorBubbleMenuProps> = (props) => {
|
||||
command: () => props.editor.chain().focus().toggleCode().run(),
|
||||
icon: IconCode,
|
||||
},
|
||||
{
|
||||
name: "Spoiler",
|
||||
isActive: () => editorState?.isSpoiler,
|
||||
command: () => props.editor.chain().focus().toggleSpoiler().run(),
|
||||
icon: IconEyeOff,
|
||||
},
|
||||
{
|
||||
name: "Clear formatting",
|
||||
// Action, not a toggle — never show an active/highlighted state.
|
||||
isActive: () => false,
|
||||
// Mirror the fixed-toolbar behavior: strip all inline marks from the selection.
|
||||
command: () => props.editor.chain().focus().unsetAllMarks().run(),
|
||||
icon: IconClearFormatting,
|
||||
},
|
||||
];
|
||||
|
||||
const commentItem: BubbleMenuItem = {
|
||||
|
||||
@@ -1,16 +1,7 @@
|
||||
import React, { useCallback, useEffect, useState } from "react";
|
||||
import { Editor } from "@tiptap/react";
|
||||
import {
|
||||
ActionIcon,
|
||||
Button,
|
||||
Group,
|
||||
Paper,
|
||||
Text,
|
||||
Textarea,
|
||||
Tooltip,
|
||||
} from "@mantine/core";
|
||||
import { IconAlt } from "@tabler/icons-react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useImageTextFieldControl } from "@/features/editor/components/common/use-image-text-field-control.tsx";
|
||||
|
||||
const ALT_MAX_LENGTH = 300;
|
||||
|
||||
@@ -27,113 +18,25 @@ type UseAltTextControlArgs = {
|
||||
currentAlt: string;
|
||||
};
|
||||
|
||||
// Thin wrapper over the shared image text-field popover; see
|
||||
// useImageTextFieldControl. The t("...") literals stay here so they remain
|
||||
// statically extractable for i18n.
|
||||
export function useAltTextControl({
|
||||
editor,
|
||||
nodeName,
|
||||
currentAlt,
|
||||
}: UseAltTextControlArgs) {
|
||||
const { t } = useTranslation();
|
||||
const [showInput, setShowInput] = useState(false);
|
||||
const [draft, setDraft] = useState("");
|
||||
|
||||
const open = useCallback(() => {
|
||||
setDraft(currentAlt || "");
|
||||
setShowInput(true);
|
||||
}, [currentAlt]);
|
||||
|
||||
useEffect(() => {
|
||||
const handler = () => {
|
||||
if (!editor.isActive(nodeName)) {
|
||||
setShowInput(false);
|
||||
}
|
||||
};
|
||||
editor.on("selectionUpdate", handler);
|
||||
return () => {
|
||||
editor.off("selectionUpdate", handler);
|
||||
};
|
||||
}, [editor, nodeName]);
|
||||
|
||||
const cancel = useCallback(() => {
|
||||
setShowInput(false);
|
||||
}, []);
|
||||
|
||||
const save = useCallback(() => {
|
||||
editor
|
||||
.chain()
|
||||
.focus(undefined, { scrollIntoView: false })
|
||||
.updateAttributes(nodeName, { alt: sanitizeAlt(draft) || undefined })
|
||||
.run();
|
||||
setShowInput(false);
|
||||
}, [editor, nodeName, draft]);
|
||||
|
||||
const onKeyDown = useCallback(
|
||||
(e: React.KeyboardEvent) => {
|
||||
if (e.key === "Enter" && (e.metaKey || e.ctrlKey)) {
|
||||
e.preventDefault();
|
||||
save();
|
||||
} else if (e.key === "Escape") {
|
||||
e.preventDefault();
|
||||
cancel();
|
||||
}
|
||||
},
|
||||
[save, cancel],
|
||||
);
|
||||
|
||||
const button = (
|
||||
<Tooltip position="top" label={t("Alt text")} withinPortal={false}>
|
||||
<ActionIcon
|
||||
onClick={open}
|
||||
size="lg"
|
||||
aria-label={t("Alt text")}
|
||||
variant="subtle"
|
||||
>
|
||||
<IconAlt size={18} />
|
||||
</ActionIcon>
|
||||
</Tooltip>
|
||||
);
|
||||
|
||||
const panel = showInput ? (
|
||||
<Paper
|
||||
withBorder
|
||||
shadow="md"
|
||||
radius={6}
|
||||
p="sm"
|
||||
w={320}
|
||||
style={{ position: "relative", zIndex: 100 }}
|
||||
>
|
||||
<Text size="sm" fw={600} mb={2}>
|
||||
{t("Alt text")}
|
||||
</Text>
|
||||
<Text size="xs" c="dimmed" mb="xs">
|
||||
{t("Describe this for accessibility.")}
|
||||
</Text>
|
||||
<Textarea
|
||||
size="xs"
|
||||
placeholder={t("Add a description")}
|
||||
value={draft}
|
||||
onChange={(e) => setDraft(e.currentTarget.value)}
|
||||
onKeyDown={onKeyDown}
|
||||
autoFocus
|
||||
autosize
|
||||
minRows={2}
|
||||
maxRows={5}
|
||||
maxLength={ALT_MAX_LENGTH}
|
||||
/>
|
||||
<Group justify="space-between" align="center" mt="xs" wrap="nowrap">
|
||||
<Text size="xs" c="dimmed">
|
||||
{draft.length}/{ALT_MAX_LENGTH}
|
||||
</Text>
|
||||
<Group gap="xs">
|
||||
<Button size="compact-xs" variant="default" onClick={cancel}>
|
||||
{t("Cancel")}
|
||||
</Button>
|
||||
<Button size="compact-xs" onClick={save}>
|
||||
{t("Save")}
|
||||
</Button>
|
||||
</Group>
|
||||
</Group>
|
||||
</Paper>
|
||||
) : null;
|
||||
|
||||
return { button, panel, isEditing: showInput };
|
||||
return useImageTextFieldControl({
|
||||
editor,
|
||||
nodeName,
|
||||
currentValue: currentAlt,
|
||||
attrName: "alt",
|
||||
sanitize: sanitizeAlt,
|
||||
maxLength: ALT_MAX_LENGTH,
|
||||
icon: <IconAlt size={18} />,
|
||||
label: t("Alt text"),
|
||||
description: t("Describe this for accessibility."),
|
||||
placeholder: t("Add a description"),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { sanitizeCaption } from "@/features/editor/components/common/use-caption-control.tsx";
|
||||
|
||||
/**
|
||||
* `sanitizeCaption` = collapse every whitespace run to a single space + trim +
|
||||
* cap at 500 chars. Captions are plain visible text, so this is a softer
|
||||
* normalization than alt-text sanitization.
|
||||
*/
|
||||
describe("sanitizeCaption", () => {
|
||||
it("trims leading and trailing whitespace", () => {
|
||||
expect(sanitizeCaption(" hello ")).toBe("hello");
|
||||
});
|
||||
|
||||
it("collapses internal whitespace runs to a single space", () => {
|
||||
expect(sanitizeCaption("a b c")).toBe("a b c");
|
||||
});
|
||||
|
||||
it("treats tab, newline and CRLF as whitespace", () => {
|
||||
expect(sanitizeCaption("a\tb")).toBe("a b");
|
||||
expect(sanitizeCaption("a\nb")).toBe("a b");
|
||||
expect(sanitizeCaption("a\r\nb")).toBe("a b");
|
||||
expect(sanitizeCaption("line1\n\n\nline2")).toBe("line1 line2");
|
||||
});
|
||||
|
||||
it("treats unicode whitespace (no-break space) as a separator", () => {
|
||||
// U+00A0 NO-BREAK SPACE is matched by the \s class.
|
||||
expect(sanitizeCaption("a b")).toBe("a b");
|
||||
});
|
||||
|
||||
it("returns empty string for whitespace-only input", () => {
|
||||
expect(sanitizeCaption(" ")).toBe("");
|
||||
expect(sanitizeCaption("")).toBe("");
|
||||
});
|
||||
|
||||
it("keeps a caption at the 500-char limit unchanged", () => {
|
||||
const exact = "x".repeat(500);
|
||||
expect(sanitizeCaption(exact)).toHaveLength(500);
|
||||
expect(sanitizeCaption(exact)).toBe(exact);
|
||||
});
|
||||
|
||||
it("slices a caption longer than 500 chars down to 500", () => {
|
||||
const tooLong = "y".repeat(600);
|
||||
const result = sanitizeCaption(tooLong);
|
||||
expect(result).toHaveLength(500);
|
||||
expect(result).toBe("y".repeat(500));
|
||||
});
|
||||
|
||||
it("collapses whitespace before applying the 500-char cap", () => {
|
||||
// 120 "a b " groups (600 raw chars) collapse to "a b a b ..." = 479 chars
|
||||
// after trimming the trailing space, which stays under the 500 cap — so only
|
||||
// the collapse is exercised here, no slice. (See the dedicated >500 test
|
||||
// above for the slice boundary.)
|
||||
const input = "a b ".repeat(120); // lots of double spaces
|
||||
const result = sanitizeCaption(input);
|
||||
expect(result).toHaveLength(479);
|
||||
expect(result.length).toBeLessThanOrEqual(500);
|
||||
expect(result).not.toMatch(/\s{2,}/);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,42 @@
|
||||
import { Editor } from "@tiptap/react";
|
||||
import { IconTextCaption } from "@tabler/icons-react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { useImageTextFieldControl } from "@/features/editor/components/common/use-image-text-field-control.tsx";
|
||||
|
||||
const CAPTION_MAX_LENGTH = 500;
|
||||
|
||||
// Caption is plain visible text (not a markdown link target like alt), so it is
|
||||
// sanitized more softly than alt: collapse runs of whitespace/newlines into a
|
||||
// single space and trim, keeping the limit generous.
|
||||
export function sanitizeCaption(value: string): string {
|
||||
return value.replace(/\s+/g, " ").trim().slice(0, CAPTION_MAX_LENGTH);
|
||||
}
|
||||
|
||||
type UseCaptionControlArgs = {
|
||||
editor: Editor;
|
||||
nodeName: string;
|
||||
currentCaption: string;
|
||||
};
|
||||
|
||||
// Thin wrapper over the shared image text-field popover; see
|
||||
// useImageTextFieldControl. The t("...") literals stay here so they remain
|
||||
// statically extractable for i18n.
|
||||
export function useCaptionControl({
|
||||
editor,
|
||||
nodeName,
|
||||
currentCaption,
|
||||
}: UseCaptionControlArgs) {
|
||||
const { t } = useTranslation();
|
||||
return useImageTextFieldControl({
|
||||
editor,
|
||||
nodeName,
|
||||
currentValue: currentCaption,
|
||||
attrName: "caption",
|
||||
sanitize: sanitizeCaption,
|
||||
maxLength: CAPTION_MAX_LENGTH,
|
||||
icon: <IconTextCaption size={18} />,
|
||||
label: t("Caption"),
|
||||
description: t("Shown below the image."),
|
||||
placeholder: t("Add a caption"),
|
||||
});
|
||||
}
|
||||
@@ -0,0 +1,145 @@
|
||||
import React, { useCallback, useEffect, useState } from "react";
|
||||
import { Editor } from "@tiptap/react";
|
||||
import {
|
||||
ActionIcon,
|
||||
Button,
|
||||
Group,
|
||||
Paper,
|
||||
Text,
|
||||
Textarea,
|
||||
Tooltip,
|
||||
} from "@mantine/core";
|
||||
import { useTranslation } from "react-i18next";
|
||||
|
||||
// Shared logic+UI for the image bubble-menu text-field popovers (alt text,
|
||||
// caption, ...). Each field is the same popover — an ActionIcon that opens a
|
||||
// titled Paper with a counted Textarea and Cancel/Save — differing only in the
|
||||
// node attribute it writes, its sanitizer, length cap, icon and labels. The
|
||||
// label/description/placeholder are passed already translated so the literal
|
||||
// t("...") calls stay in the thin wrappers and remain extractable; the shared
|
||||
// Cancel/Save strings are translated here.
|
||||
type UseImageTextFieldControlArgs = {
|
||||
editor: Editor;
|
||||
nodeName: string;
|
||||
currentValue: string;
|
||||
attrName: string;
|
||||
sanitize: (value: string) => string;
|
||||
maxLength: number;
|
||||
icon: React.ReactNode;
|
||||
label: string;
|
||||
description: string;
|
||||
placeholder: string;
|
||||
};
|
||||
|
||||
export function useImageTextFieldControl({
|
||||
editor,
|
||||
nodeName,
|
||||
currentValue,
|
||||
attrName,
|
||||
sanitize,
|
||||
maxLength,
|
||||
icon,
|
||||
label,
|
||||
description,
|
||||
placeholder,
|
||||
}: UseImageTextFieldControlArgs) {
|
||||
const { t } = useTranslation();
|
||||
const [showInput, setShowInput] = useState(false);
|
||||
const [draft, setDraft] = useState("");
|
||||
|
||||
const open = useCallback(() => {
|
||||
setDraft(currentValue || "");
|
||||
setShowInput(true);
|
||||
}, [currentValue]);
|
||||
|
||||
useEffect(() => {
|
||||
const handler = () => {
|
||||
if (!editor.isActive(nodeName)) {
|
||||
setShowInput(false);
|
||||
}
|
||||
};
|
||||
editor.on("selectionUpdate", handler);
|
||||
return () => {
|
||||
editor.off("selectionUpdate", handler);
|
||||
};
|
||||
}, [editor, nodeName]);
|
||||
|
||||
const cancel = useCallback(() => {
|
||||
setShowInput(false);
|
||||
}, []);
|
||||
|
||||
const save = useCallback(() => {
|
||||
editor
|
||||
.chain()
|
||||
.focus(undefined, { scrollIntoView: false })
|
||||
.updateAttributes(nodeName, { [attrName]: sanitize(draft) || undefined })
|
||||
.run();
|
||||
setShowInput(false);
|
||||
}, [editor, nodeName, attrName, sanitize, draft]);
|
||||
|
||||
const onKeyDown = useCallback(
|
||||
(e: React.KeyboardEvent) => {
|
||||
if (e.key === "Enter" && (e.metaKey || e.ctrlKey)) {
|
||||
e.preventDefault();
|
||||
save();
|
||||
} else if (e.key === "Escape") {
|
||||
e.preventDefault();
|
||||
cancel();
|
||||
}
|
||||
},
|
||||
[save, cancel],
|
||||
);
|
||||
|
||||
const button = (
|
||||
<Tooltip position="top" label={label} withinPortal={false}>
|
||||
<ActionIcon onClick={open} size="lg" aria-label={label} variant="subtle">
|
||||
{icon}
|
||||
</ActionIcon>
|
||||
</Tooltip>
|
||||
);
|
||||
|
||||
const panel = showInput ? (
|
||||
<Paper
|
||||
withBorder
|
||||
shadow="md"
|
||||
radius={6}
|
||||
p="sm"
|
||||
w={320}
|
||||
style={{ position: "relative", zIndex: 100 }}
|
||||
>
|
||||
<Text size="sm" fw={600} mb={2}>
|
||||
{label}
|
||||
</Text>
|
||||
<Text size="xs" c="dimmed" mb="xs">
|
||||
{description}
|
||||
</Text>
|
||||
<Textarea
|
||||
size="xs"
|
||||
placeholder={placeholder}
|
||||
value={draft}
|
||||
onChange={(e) => setDraft(e.currentTarget.value)}
|
||||
onKeyDown={onKeyDown}
|
||||
autoFocus
|
||||
autosize
|
||||
minRows={2}
|
||||
maxRows={5}
|
||||
maxLength={maxLength}
|
||||
/>
|
||||
<Group justify="space-between" align="center" mt="xs" wrap="nowrap">
|
||||
<Text size="xs" c="dimmed">
|
||||
{draft.length}/{maxLength}
|
||||
</Text>
|
||||
<Group gap="xs">
|
||||
<Button size="compact-xs" variant="default" onClick={cancel}>
|
||||
{t("Cancel")}
|
||||
</Button>
|
||||
<Button size="compact-xs" onClick={save}>
|
||||
{t("Save")}
|
||||
</Button>
|
||||
</Group>
|
||||
</Group>
|
||||
</Paper>
|
||||
) : null;
|
||||
|
||||
return { button, panel, isEditing: showInput };
|
||||
}
|
||||
@@ -23,6 +23,7 @@ import { useTranslation } from "react-i18next";
|
||||
import { getFileUrl } from "@/lib/config.ts";
|
||||
import { uploadImageAction } from "@/features/editor/components/image/upload-image-action.tsx";
|
||||
import { useAltTextControl } from "@/features/editor/components/common/use-alt-text-control.tsx";
|
||||
import { useCaptionControl } from "@/features/editor/components/common/use-caption-control.tsx";
|
||||
import classes from "../common/toolbar-menu.module.css";
|
||||
|
||||
export function ImageMenu({ editor }: EditorMenuProps) {
|
||||
@@ -47,6 +48,7 @@ export function ImageMenu({ editor }: EditorMenuProps) {
|
||||
isFloatRight: ctx.editor.isActive("image", { align: "floatRight" }),
|
||||
src: imageAttrs?.src || null,
|
||||
alt: imageAttrs?.alt || "",
|
||||
caption: imageAttrs?.caption || "",
|
||||
};
|
||||
},
|
||||
});
|
||||
@@ -168,6 +170,16 @@ export function ImageMenu({ editor }: EditorMenuProps) {
|
||||
currentAlt: editorState?.alt || "",
|
||||
});
|
||||
|
||||
const {
|
||||
button: captionButton,
|
||||
panel: captionPanel,
|
||||
isEditing: isEditingCaption,
|
||||
} = useCaptionControl({
|
||||
editor,
|
||||
nodeName: "image",
|
||||
currentCaption: editorState?.caption || "",
|
||||
});
|
||||
|
||||
return (
|
||||
<BaseBubbleMenu
|
||||
editor={editor}
|
||||
@@ -183,6 +195,8 @@ export function ImageMenu({ editor }: EditorMenuProps) {
|
||||
>
|
||||
{isEditingAlt ? (
|
||||
altTextPanel
|
||||
) : isEditingCaption ? (
|
||||
captionPanel
|
||||
) : (
|
||||
<div className={classes.toolbar}>
|
||||
<Tooltip position="top" label={t("Align left")} withinPortal={false}>
|
||||
@@ -249,6 +263,8 @@ export function ImageMenu({ editor }: EditorMenuProps) {
|
||||
|
||||
{altTextButton}
|
||||
|
||||
{captionButton}
|
||||
|
||||
<div className={classes.divider} />
|
||||
|
||||
<Tooltip position="top" label={t("Download")} withinPortal={false}>
|
||||
|
||||
@@ -9,7 +9,9 @@ import { useTranslation } from "react-i18next";
|
||||
export default function ImageView(props: NodeViewProps) {
|
||||
const { t } = useTranslation();
|
||||
const { editor, node, selected } = props;
|
||||
const { src, width, align, alt, aspectRatio, placeholder } = node.attrs;
|
||||
const { src, width, align, alt, caption, aspectRatio, placeholder } =
|
||||
node.attrs;
|
||||
const captionText = (caption || "").trim();
|
||||
const alignClass = useMemo(() => {
|
||||
if (align === "left") return "alignLeft";
|
||||
if (align === "right") return "alignRight";
|
||||
@@ -29,6 +31,7 @@ export default function ImageView(props: NodeViewProps) {
|
||||
|
||||
return (
|
||||
<NodeViewWrapper data-drag-handle>
|
||||
<figure style={{ margin: 0 }}>
|
||||
<div
|
||||
className={clsx(
|
||||
selected && "ProseMirror-selectednode",
|
||||
@@ -66,6 +69,15 @@ export default function ImageView(props: NodeViewProps) {
|
||||
</Group>
|
||||
)}
|
||||
</div>
|
||||
{captionText && (
|
||||
<Text
|
||||
component="figcaption"
|
||||
className="image-caption"
|
||||
>
|
||||
{captionText}
|
||||
</Text>
|
||||
)}
|
||||
</figure>
|
||||
</NodeViewWrapper>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,194 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
// Mock the page-service so importing the module under test does not pull in the
|
||||
// axios/api-client chain. `createMentionAction` is wired to `getPageById`; the
|
||||
// spy lets us assert that wiring without any network. `vi.hoisted` keeps the spy
|
||||
// available inside the hoisted vi.mock factory.
|
||||
const { getPageById } = vi.hoisted(() => ({ getPageById: vi.fn() }));
|
||||
vi.mock("@/features/page/services/page-service.ts", () => ({
|
||||
getPageById,
|
||||
}));
|
||||
|
||||
// `uuid` v7 is used for the mention node id; pin only v7 so assertions are
|
||||
// stable, keeping the rest (e.g. `validate`, used by extractPageSlugId) real.
|
||||
vi.mock("uuid", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("uuid")>()),
|
||||
v7: () => "fixed-mention-uuid",
|
||||
}));
|
||||
|
||||
import {
|
||||
handleInternalLink,
|
||||
createMentionAction,
|
||||
} from "./internal-link-paste";
|
||||
|
||||
// Minimal ProseMirror-ish EditorView fake. We record what handleInternalLink
|
||||
// builds and dispatches without standing up a real schema/state.
|
||||
function makeView() {
|
||||
const tr = {
|
||||
replaceWith: vi.fn(function (this: unknown) {
|
||||
return tr;
|
||||
}),
|
||||
insertText: vi.fn(function (this: unknown) {
|
||||
return tr;
|
||||
}),
|
||||
addMark: vi.fn(function (this: unknown) {
|
||||
return tr;
|
||||
}),
|
||||
};
|
||||
const schema = {
|
||||
nodes: {
|
||||
mention: {
|
||||
// Echo the attrs back so we can assert exactly what was created.
|
||||
create: vi.fn((attrs: Record<string, unknown>) => ({
|
||||
type: "mention",
|
||||
attrs,
|
||||
})),
|
||||
},
|
||||
},
|
||||
marks: {
|
||||
link: {
|
||||
create: vi.fn((attrs: Record<string, unknown>) => ({
|
||||
type: "link",
|
||||
attrs,
|
||||
})),
|
||||
},
|
||||
},
|
||||
};
|
||||
const view = {
|
||||
state: { schema, tr },
|
||||
dispatch: vi.fn(),
|
||||
};
|
||||
return { view, tr, schema };
|
||||
}
|
||||
|
||||
describe("handleInternalLink", () => {
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
it("does nothing when validateFn rejects the url (no resolve, no dispatch)", async () => {
|
||||
const onResolveLink = vi.fn();
|
||||
const validateFn = vi.fn(() => false);
|
||||
const { view } = makeView();
|
||||
|
||||
await handleInternalLink({ validateFn, onResolveLink })(
|
||||
"any-url",
|
||||
view as never,
|
||||
3,
|
||||
"creator-1",
|
||||
);
|
||||
|
||||
expect(validateFn).toHaveBeenCalledWith("any-url", view);
|
||||
expect(onResolveLink).not.toHaveBeenCalled();
|
||||
expect(view.dispatch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("on resolve: inserts a mention node carrying the resolved page + anchor and dispatches replaceWith at pos", async () => {
|
||||
const page = {
|
||||
id: "page-id-99",
|
||||
title: "My Page",
|
||||
slugId: "slugABC",
|
||||
};
|
||||
const onResolveLink = vi.fn().mockResolvedValue(page);
|
||||
const { view, tr, schema } = makeView();
|
||||
|
||||
// extractPageSlugId("doc-slug-xyz789") -> "xyz789" (last hyphen segment).
|
||||
await handleInternalLink({ validateFn: () => true, onResolveLink })(
|
||||
"doc-slug-xyz789",
|
||||
view as never,
|
||||
5,
|
||||
"creator-7",
|
||||
"anchor-42",
|
||||
);
|
||||
|
||||
// The linked page id is the extracted slug-id, not the whole url.
|
||||
expect(onResolveLink).toHaveBeenCalledWith("xyz789", "creator-7");
|
||||
expect(schema.nodes.mention.create).toHaveBeenCalledWith({
|
||||
id: "fixed-mention-uuid",
|
||||
label: "My Page",
|
||||
entityType: "page",
|
||||
entityId: "page-id-99",
|
||||
slugId: "slugABC",
|
||||
creatorId: "creator-7",
|
||||
anchorId: "anchor-42",
|
||||
});
|
||||
expect(tr.replaceWith).toHaveBeenCalledWith(5, 5, {
|
||||
type: "mention",
|
||||
attrs: expect.objectContaining({ entityId: "page-id-99" }),
|
||||
});
|
||||
expect(tr.insertText).not.toHaveBeenCalled();
|
||||
expect(view.dispatch).toHaveBeenCalledTimes(1);
|
||||
expect(view.dispatch).toHaveBeenCalledWith(tr);
|
||||
});
|
||||
|
||||
it("falls back to 'Untitled' label when the resolved page has no title", async () => {
|
||||
const onResolveLink = vi
|
||||
.fn()
|
||||
.mockResolvedValue({ id: "p", title: "", slugId: "s" });
|
||||
const { view, schema } = makeView();
|
||||
|
||||
await handleInternalLink({ validateFn: () => true, onResolveLink })(
|
||||
"abc-id1",
|
||||
view as never,
|
||||
0,
|
||||
"c",
|
||||
);
|
||||
|
||||
expect(schema.nodes.mention.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ label: "Untitled" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("on reject: inserts the raw url as plain text with a link mark and dispatches", async () => {
|
||||
const onResolveLink = vi.fn().mockRejectedValue(new Error("not found"));
|
||||
const { view, tr, schema } = makeView();
|
||||
|
||||
await handleInternalLink({ validateFn: () => true, onResolveLink })(
|
||||
"http://x/page-id2",
|
||||
view as never,
|
||||
4,
|
||||
"creator-1",
|
||||
);
|
||||
|
||||
// No mention node on the failure path.
|
||||
expect(schema.nodes.mention.create).not.toHaveBeenCalled();
|
||||
expect(tr.insertText).toHaveBeenCalledWith("http://x/page-id2", 4);
|
||||
expect(schema.marks.link.create).toHaveBeenCalledWith({
|
||||
href: "http://x/page-id2",
|
||||
});
|
||||
// Mark spans exactly the inserted url text: [pos, pos + url.length].
|
||||
expect(tr.addMark).toHaveBeenCalledWith(4, 4 + "http://x/page-id2".length, {
|
||||
type: "link",
|
||||
attrs: { href: "http://x/page-id2" },
|
||||
});
|
||||
expect(view.dispatch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("createMentionAction", () => {
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
it("resolves the link via getPageById and inserts the mention", async () => {
|
||||
getPageById.mockResolvedValue({
|
||||
id: "real-page",
|
||||
title: "Real",
|
||||
slugId: "rslug",
|
||||
});
|
||||
const { view, schema } = makeView();
|
||||
|
||||
await createMentionAction("ref-pageABC", view as never, 2, "creator-9");
|
||||
|
||||
expect(getPageById).toHaveBeenCalledWith({ pageId: "pageABC" });
|
||||
expect(schema.nodes.mention.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ entityId: "real-page", label: "Real" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("propagates a getPageById failure to the plain-link fallback", async () => {
|
||||
getPageById.mockRejectedValue(new Error("404"));
|
||||
const { view, tr } = makeView();
|
||||
|
||||
await createMentionAction("ref-pageABC", view as never, 1, "creator-9");
|
||||
|
||||
// Failure path: the url is inserted as text, not as a mention node.
|
||||
expect(tr.insertText).toHaveBeenCalledWith("ref-pageABC", 1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,20 @@
|
||||
import { MarkViewContent, MarkViewProps } from "@tiptap/react";
|
||||
import { useState } from "react";
|
||||
|
||||
// Click-to-reveal spoiler. The revealed state is UI-only and is never written to
|
||||
// the document: toggling only adds/removes the `is-revealed` class (CSS removes
|
||||
// the blur). renderHTML never emits `is-revealed`, so it can't leak into the
|
||||
// doc/clipboard. Works the same in editor, read-only and public-share views.
|
||||
export default function SpoilerView(_props: MarkViewProps) {
|
||||
const [revealed, setRevealed] = useState(false);
|
||||
|
||||
return (
|
||||
<span
|
||||
className={revealed ? "spoiler is-revealed" : "spoiler"}
|
||||
data-spoiler="true"
|
||||
onClick={() => setRevealed((v) => !v)}
|
||||
>
|
||||
<MarkViewContent />
|
||||
</span>
|
||||
);
|
||||
}
|
||||
@@ -53,6 +53,7 @@ import {
|
||||
Subpages,
|
||||
Heading,
|
||||
Highlight,
|
||||
Spoiler,
|
||||
Indent,
|
||||
UniqueID,
|
||||
SharedStorage,
|
||||
@@ -116,6 +117,7 @@ import mentionRenderItems from "@/features/editor/components/mention/mention-sug
|
||||
import { ReactNodeViewRenderer, ReactMarkViewRenderer } from "@tiptap/react";
|
||||
import MentionView from "@/features/editor/components/mention/mention-view.tsx";
|
||||
import LinkView from "@/features/editor/components/link/link-view.tsx";
|
||||
import SpoilerView from "@/features/editor/components/spoiler/spoiler-view.tsx";
|
||||
import i18n from "@/i18n.ts";
|
||||
import { MarkdownClipboard } from "@/features/editor/extensions/markdown-clipboard.ts";
|
||||
import EmojiCommand from "./emoji-command";
|
||||
@@ -123,6 +125,7 @@ import { countWords } from "alfaaz";
|
||||
import AutoJoiner from "@/features/editor/extensions/autojoiner.ts";
|
||||
import GlobalDragHandle from "@/features/editor/extensions/drag-handle.ts";
|
||||
import { CleanStyles } from "@/features/editor/extensions/clean-styles.ts";
|
||||
import { IntentionalClear } from "@/features/editor/extensions/intentional-clear.ts";
|
||||
|
||||
const lowlight = createLowlight(common);
|
||||
lowlight.register("mermaid", plaintext);
|
||||
@@ -237,6 +240,11 @@ export const mainExtensions = [
|
||||
Highlight.configure({
|
||||
multicolor: true,
|
||||
}),
|
||||
Spoiler.configure({}).extend({
|
||||
addMarkView() {
|
||||
return ReactMarkViewRenderer(SpoilerView);
|
||||
},
|
||||
}),
|
||||
Typography,
|
||||
TrailingNode,
|
||||
GlobalDragHandle.configure({
|
||||
@@ -486,4 +494,10 @@ export const collabExtensions: CollabExtensions = (provider, user) => [
|
||||
color: randomElement(userColors),
|
||||
},
|
||||
}),
|
||||
// #251 — emit an intentional-clear signal to the server when the user
|
||||
// deliberately empties the page, so the #248 store-side empty-guard lets that
|
||||
// one clear through while still blocking accidental empties.
|
||||
IntentionalClear.configure({
|
||||
provider,
|
||||
}),
|
||||
];
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { Editor } from "@tiptap/core";
|
||||
import { Document } from "@tiptap/extension-document";
|
||||
import { Paragraph } from "@tiptap/extension-paragraph";
|
||||
import { Text } from "@tiptap/extension-text";
|
||||
import { ySyncPluginKey } from "@tiptap/y-tiptap";
|
||||
import {
|
||||
IntentionalClear,
|
||||
INTENTIONAL_CLEAR_MESSAGE_TYPE,
|
||||
} from "./intentional-clear";
|
||||
|
||||
/**
|
||||
* #251 — the intentional-clear signal is driven through the REAL editor path:
|
||||
* a fresh Editor with the IntentionalClear extension, a fake provider that
|
||||
* records sendStateless, and the actual select-all + delete command the user's
|
||||
* keystroke runs. No hand-poke of any flag.
|
||||
*/
|
||||
describe("IntentionalClear extension", () => {
|
||||
let sendStateless: ReturnType<typeof vi.fn>;
|
||||
|
||||
const makeEditor = (content: unknown) =>
|
||||
new Editor({
|
||||
extensions: [
|
||||
Document,
|
||||
Paragraph,
|
||||
Text,
|
||||
IntentionalClear.configure({
|
||||
// Minimal provider stand-in: only sendStateless is exercised.
|
||||
provider: { sendStateless } as any,
|
||||
}),
|
||||
],
|
||||
content: content as any,
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
sendStateless = vi.fn();
|
||||
});
|
||||
|
||||
it("emits the clear signal when a user empties a non-empty doc (select-all + delete)", () => {
|
||||
const editor = makeEditor({
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "hello world" }] },
|
||||
],
|
||||
});
|
||||
|
||||
// The exact command path a select-all + Delete keystroke dispatches.
|
||||
editor.chain().selectAll().deleteSelection().run();
|
||||
|
||||
expect(sendStateless).toHaveBeenCalledTimes(1);
|
||||
const payload = JSON.parse(sendStateless.mock.calls[0][0]);
|
||||
expect(payload).toEqual({ type: INTENTIONAL_CLEAR_MESSAGE_TYPE });
|
||||
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
it("does NOT emit when typing into an empty doc (no non-empty → empty transition)", () => {
|
||||
const editor = makeEditor({ type: "doc", content: [{ type: "paragraph" }] });
|
||||
|
||||
editor.chain().insertContent("typed text").run();
|
||||
|
||||
expect(sendStateless).not.toHaveBeenCalled();
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
it("does NOT emit on an edit that leaves the doc non-empty", () => {
|
||||
const editor = makeEditor({
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "keep me" }] },
|
||||
],
|
||||
});
|
||||
|
||||
editor.chain().insertContent(" more").run();
|
||||
|
||||
expect(sendStateless).not.toHaveBeenCalled();
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
it("does NOT emit when a REMOTE/merge (change-origin) transaction empties the doc", () => {
|
||||
// This pins the CENTRAL #248 protection: only a LOCAL user edit may emit the
|
||||
// intentional-clear signal. An emptiness arriving from another client, a bad
|
||||
// merge, or an emptied transclusion is applied as a y-sync transaction tagged
|
||||
// with the ySyncPluginKey meta, which `isChangeOrigin` detects. The extension
|
||||
// must early-return on it and NOT punch the empty write through the server
|
||||
// guard.
|
||||
const editor = makeEditor({
|
||||
type: "doc",
|
||||
content: [
|
||||
{ type: "paragraph", content: [{ type: "text", text: "remote content" }] },
|
||||
],
|
||||
});
|
||||
|
||||
// Build a transaction that empties the non-empty doc and tag it exactly the
|
||||
// way y-tiptap tags a remote y-sync update: `tr.setMeta(ySyncPluginKey,
|
||||
// { isChangeOrigin: true })` (see @tiptap/y-tiptap sync-plugin). This makes
|
||||
// the real `isChangeOrigin(tr)` predicate return true — not a stand-in.
|
||||
const { state } = editor;
|
||||
const tr = state.tr
|
||||
.delete(0, state.doc.content.size)
|
||||
.setMeta(ySyncPluginKey, { isChangeOrigin: true });
|
||||
editor.view.dispatch(tr);
|
||||
|
||||
// The transaction really emptied the doc (became the single empty paragraph)…
|
||||
expect(editor.state.doc.textContent).toBe("");
|
||||
// …yet because it is change-origin, no signal is emitted.
|
||||
expect(sendStateless).not.toHaveBeenCalled();
|
||||
editor.destroy();
|
||||
});
|
||||
|
||||
it("does NOT emit when the doc was already empty", () => {
|
||||
const editor = makeEditor({ type: "doc", content: [{ type: "paragraph" }] });
|
||||
|
||||
// Selecting all + delete on an already-empty doc is a no-op transition.
|
||||
editor.chain().selectAll().deleteSelection().run();
|
||||
|
||||
expect(sendStateless).not.toHaveBeenCalled();
|
||||
editor.destroy();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,94 @@
|
||||
import { Extension } from "@tiptap/core";
|
||||
import { isChangeOrigin } from "@tiptap/extension-collaboration";
|
||||
import type { Node as PMNode } from "@tiptap/pm/model";
|
||||
import type { HocuspocusProvider } from "@hocuspocus/provider";
|
||||
|
||||
/**
|
||||
* Stateless message type sent to the server when a user deliberately clears a
|
||||
* page to empty. Kept in one place so the client emitter and the server
|
||||
* consumer (PersistenceExtension.onStateless) agree on the wire format.
|
||||
*/
|
||||
export const INTENTIONAL_CLEAR_MESSAGE_TYPE = "intentional-clear";
|
||||
|
||||
export interface IntentionalClearOptions {
|
||||
/** The collab provider used to send the stateless clear signal. */
|
||||
provider: HocuspocusProvider | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* A "document is empty" check that mirrors the server's `isEmptyParagraphDoc`
|
||||
* (collaboration.util.ts): exactly one top-level paragraph with no inline
|
||||
* content. After a select-all + delete TipTap leaves precisely this shape, so
|
||||
* matching it here keeps the client signal aligned with the server guard that
|
||||
* consumes it.
|
||||
*/
|
||||
function isEmptyParagraphDoc(doc: PMNode): boolean {
|
||||
if (doc.childCount !== 1) return false;
|
||||
const child = doc.firstChild;
|
||||
return (
|
||||
child !== null &&
|
||||
child !== undefined &&
|
||||
child.type.name === "paragraph" &&
|
||||
child.content.size === 0
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* #251 — intentional-clear signal.
|
||||
*
|
||||
* The server's #248 store-side empty-guard unconditionally refuses to overwrite
|
||||
* non-empty persisted content with an empty document, because a momentarily
|
||||
* empty live Y.Doc (a glitch, a bad merge, an emptying transclusion) is
|
||||
* indistinguishable from a real clear *at the store layer*. That protection is
|
||||
* correct, but it also blocks a user who genuinely wants to empty the page.
|
||||
*
|
||||
* This extension supplies the missing distinction. It watches LOCAL, user-driven
|
||||
* transactions and, the moment one reduces a non-empty document to the empty
|
||||
* single-paragraph shape, it sends a hocuspocus stateless message to the server.
|
||||
* The server records a short-lived, single-use "intentional clear pending" flag
|
||||
* for this document that the next (debounced) onStoreDocument consumes to let
|
||||
* that one empty write through the guard.
|
||||
*
|
||||
* What counts as an intentional clear (precise definition):
|
||||
* - the transaction actually changed the document (`docChanged`), AND
|
||||
* - it is a LOCAL user edit, not a remote collab application — remote y-sync
|
||||
* transactions are tagged and filtered out via `isChangeOrigin`, so an
|
||||
* emptiness that arrives from another client / a merge never emits a signal,
|
||||
* AND
|
||||
* - the document was non-empty before the transaction and is the empty
|
||||
* single-paragraph doc after it.
|
||||
*
|
||||
* This is exactly the select-all + Delete / Backspace (or any local command that
|
||||
* empties the doc, e.g. clearContent) keystroke path. A transient/programmatic
|
||||
* empty serialization that the server might see on the wire does NOT come with
|
||||
* this signal, so the guard still blocks it.
|
||||
*/
|
||||
export const IntentionalClear = Extension.create<IntentionalClearOptions>({
|
||||
name: "intentionalClear",
|
||||
|
||||
addOptions() {
|
||||
return {
|
||||
provider: null,
|
||||
};
|
||||
},
|
||||
|
||||
onTransaction({ transaction }) {
|
||||
if (!transaction.docChanged) return;
|
||||
// Only react to local user edits. Remote collaboration steps (and other
|
||||
// y-sync-applied changes) carry the change origin and must never be treated
|
||||
// as an intentional clear, otherwise a remote/merge-induced emptiness would
|
||||
// punch through the server guard.
|
||||
if (isChangeOrigin(transaction)) return;
|
||||
|
||||
const becameEmpty =
|
||||
!isEmptyParagraphDoc(transaction.before) &&
|
||||
isEmptyParagraphDoc(transaction.doc);
|
||||
if (!becameEmpty) return;
|
||||
|
||||
// The server reads the originating document from the connection, so the
|
||||
// payload only needs to declare intent — it cannot target another document.
|
||||
this.options.provider?.sendStateless(
|
||||
JSON.stringify({ type: INTENTIONAL_CLEAR_MESSAGE_TYPE }),
|
||||
);
|
||||
},
|
||||
});
|
||||
@@ -14,6 +14,7 @@
|
||||
@import "./mention.css";
|
||||
@import "./ordered-list.css";
|
||||
@import "./highlight.css";
|
||||
@import "./spoiler.css";
|
||||
@import "./indent.css";
|
||||
@import "./columns.css";
|
||||
@import "./status.css";
|
||||
|
||||
@@ -33,6 +33,15 @@
|
||||
}
|
||||
}
|
||||
|
||||
.image-caption {
|
||||
text-align: center;
|
||||
font-size: 0.875em;
|
||||
color: var(--mantine-color-dimmed);
|
||||
margin-top: 0.4em;
|
||||
line-height: 1.35;
|
||||
word-break: break-word;
|
||||
}
|
||||
|
||||
.uploading-text {
|
||||
font-size: var(--mantine-font-size-md);
|
||||
line-height: var(--mantine-line-height-md);
|
||||
|
||||
21
apps/client/src/features/editor/styles/spoiler.css
Normal file
21
apps/client/src/features/editor/styles/spoiler.css
Normal file
@@ -0,0 +1,21 @@
|
||||
.spoiler {
|
||||
background: rgba(0, 0, 0, 0.85);
|
||||
border-radius: 0.25em;
|
||||
cursor: pointer;
|
||||
filter: blur(0.3em);
|
||||
transition: filter 0.15s ease;
|
||||
user-select: none;
|
||||
}
|
||||
|
||||
.spoiler.is-revealed {
|
||||
filter: none;
|
||||
background: rgba(125, 125, 125, 0.18);
|
||||
user-select: auto;
|
||||
}
|
||||
|
||||
@media print {
|
||||
.spoiler {
|
||||
filter: none;
|
||||
background: rgba(125, 125, 125, 0.18);
|
||||
}
|
||||
}
|
||||
@@ -3,6 +3,9 @@ import {
|
||||
resolveCardStatus,
|
||||
isEndpointConfigured,
|
||||
resolveKeyField,
|
||||
nextReindexPollInterval,
|
||||
isReindexComplete,
|
||||
isReindexButtonLoading,
|
||||
} from './ai-provider-settings';
|
||||
|
||||
describe('resolveCardStatus', () => {
|
||||
@@ -71,3 +74,152 @@ describe('resolveKeyField (write-only key payload)', () => {
|
||||
expect(resolveKeyField('', false)).toEqual({ set: false });
|
||||
});
|
||||
});
|
||||
|
||||
describe('nextReindexPollInterval', () => {
|
||||
const INTERVAL = 5000;
|
||||
const base = { now: 1_000, intervalMs: INTERVAL };
|
||||
|
||||
it('does not poll when no reindex deadline is set', () => {
|
||||
expect(
|
||||
nextReindexPollInterval({
|
||||
...base,
|
||||
deadline: null,
|
||||
status: { reindexing: true, indexedPages: 0, totalPages: 478 },
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps polling while the server reports an active run', () => {
|
||||
expect(
|
||||
nextReindexPollInterval({
|
||||
...base,
|
||||
deadline: 10_000,
|
||||
status: { reindexing: true, indexedPages: 120, totalPages: 478 },
|
||||
}),
|
||||
).toBe(INTERVAL);
|
||||
});
|
||||
|
||||
it('keeps polling during an active run even if counts momentarily look full', () => {
|
||||
// The run clears its progress record only at the very end, so a transient
|
||||
// indexed==total while reindexing is still true must NOT stop polling.
|
||||
expect(
|
||||
nextReindexPollInterval({
|
||||
...base,
|
||||
deadline: 10_000,
|
||||
status: { reindexing: true, indexedPages: 478, totalPages: 478 },
|
||||
}),
|
||||
).toBe(INTERVAL);
|
||||
});
|
||||
|
||||
it('stops once the run is finished AND fully indexed', () => {
|
||||
expect(
|
||||
nextReindexPollInterval({
|
||||
...base,
|
||||
deadline: 10_000,
|
||||
status: { reindexing: false, indexedPages: 478, totalPages: 478 },
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps polling within the deadline when not yet done and no active flag', () => {
|
||||
// First poll right after enqueue, before the worker publishes progress.
|
||||
expect(
|
||||
nextReindexPollInterval({
|
||||
...base,
|
||||
deadline: 10_000,
|
||||
status: { reindexing: false, indexedPages: 0, totalPages: 478 },
|
||||
}),
|
||||
).toBe(INTERVAL);
|
||||
});
|
||||
|
||||
it('cap always wins: stops once past the deadline even if still reindexing', () => {
|
||||
expect(
|
||||
nextReindexPollInterval({
|
||||
deadline: 1_000,
|
||||
now: 2_000, // past the deadline
|
||||
intervalMs: INTERVAL,
|
||||
status: { reindexing: true, indexedPages: 200, totalPages: 478 },
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('stops on an empty workspace (0 of 0) once the run is finished', () => {
|
||||
expect(
|
||||
nextReindexPollInterval({
|
||||
...base,
|
||||
deadline: 10_000,
|
||||
status: { reindexing: false, indexedPages: 0, totalPages: 0 },
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isReindexComplete', () => {
|
||||
it('false when no status yet', () => {
|
||||
expect(isReindexComplete(undefined)).toBe(false);
|
||||
});
|
||||
|
||||
it('false while a run is still active (even at indexed==total)', () => {
|
||||
expect(
|
||||
isReindexComplete({ reindexing: true, indexedPages: 478, totalPages: 478 }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('false when finished but not yet fully indexed', () => {
|
||||
expect(
|
||||
isReindexComplete({ reindexing: false, indexedPages: 120, totalPages: 478 }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('true once finished and fully indexed', () => {
|
||||
expect(
|
||||
isReindexComplete({ reindexing: false, indexedPages: 478, totalPages: 478 }),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isReindexButtonLoading', () => {
|
||||
it('loads while the POST mutation is pending', () => {
|
||||
expect(
|
||||
isReindexButtonLoading({
|
||||
mutationPending: true,
|
||||
deadline: null,
|
||||
status: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('does NOT load post-cap: deadline nulled but reindexing left stale-true', () => {
|
||||
// The key case: after the poll cap fires `reindexDeadline` is null while
|
||||
// `settings.reindexing` can be a stale `true` from the last poll. Gating on
|
||||
// the deadline keeps the spinner from sticking forever so the admin can
|
||||
// restart.
|
||||
expect(
|
||||
isReindexButtonLoading({
|
||||
mutationPending: false,
|
||||
deadline: null,
|
||||
status: true,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('loads during an active run within the poll window', () => {
|
||||
expect(
|
||||
isReindexButtonLoading({
|
||||
mutationPending: false,
|
||||
deadline: 10_000,
|
||||
status: true,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('does not load once the run finished while still polling', () => {
|
||||
expect(
|
||||
isReindexButtonLoading({
|
||||
mutationPending: false,
|
||||
deadline: 10_000,
|
||||
status: false,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -37,6 +37,7 @@ import {
|
||||
} from "@/features/workspace/queries/ai-settings-query.ts";
|
||||
import {
|
||||
AiTestCapability,
|
||||
IAiSettings,
|
||||
IAiSettingsUpdate,
|
||||
SttApiStyle,
|
||||
ChatApiStyle,
|
||||
@@ -169,6 +170,73 @@ export function resolveKeyField(
|
||||
return { set: false };
|
||||
}
|
||||
|
||||
// Subset of the status payload that drives the reindex poll decisions.
|
||||
type ReindexStatus = Pick<
|
||||
IAiSettings,
|
||||
"reindexing" | "indexedPages" | "totalPages"
|
||||
>;
|
||||
|
||||
/**
|
||||
* Decide the TanStack Query `refetchInterval` while a reindex may be running.
|
||||
* Returns the poll interval (ms) to keep polling, or `false` to stop.
|
||||
*
|
||||
* Polls while the server reports an ACTIVE run (`reindexing === true`) OR we are
|
||||
* still within the deadline window and not yet fully indexed. Stops once the run
|
||||
* has finished AND everything is indexed (server cleared its progress record and
|
||||
* fell back to the DB coverage count), or the deadline cap is hit — the cap
|
||||
* always wins so a stuck/never-clearing progress record can't poll forever.
|
||||
*/
|
||||
export function nextReindexPollInterval(args: {
|
||||
deadline: number | null;
|
||||
now: number;
|
||||
intervalMs: number;
|
||||
status?: ReindexStatus;
|
||||
}): number | false {
|
||||
const { deadline, now, intervalMs, status } = args;
|
||||
if (deadline === null) return false;
|
||||
// Cap always wins.
|
||||
if (now > deadline) return false;
|
||||
// Active run → keep polling even if the momentary counts already look full.
|
||||
if (status?.reindexing) return intervalMs;
|
||||
// Finished and fully indexed (incl. an empty workspace, 0 >= 0) → stop. Reuse
|
||||
// isReindexComplete so the completeness check lives in exactly one place.
|
||||
if (isReindexComplete(status)) return false;
|
||||
// Within the deadline and not yet done → keep polling.
|
||||
return intervalMs;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the reindex poll deadline should be cleared: the server reports no
|
||||
* active run AND the count is complete. The single source of truth for the
|
||||
* "reindex finished" check — `nextReindexPollInterval` reuses it for its stop
|
||||
* condition (sans the cap, which the effect handles via time).
|
||||
*/
|
||||
export function isReindexComplete(status?: ReindexStatus): boolean {
|
||||
return (
|
||||
!!status && !status.reindexing && status.indexedPages >= status.totalPages
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the reindex button should show its spinner (and stay disabled).
|
||||
*
|
||||
* Spins while the POST is in flight, and for the WHOLE background run while the
|
||||
* server reports `reindexing === true`. The `deadline !== null` gate is the
|
||||
* load-bearing part: once the 120s poll cap fires it nulls `reindexDeadline`
|
||||
* and stops refetching, so `status` (settings?.reindexing) can be a stale
|
||||
* `true` from the last poll. Without the gate the spinner would stick forever
|
||||
* for a run that outlives the cap and block a restart; gating on the active
|
||||
* poll window clears it so the admin can re-trigger.
|
||||
*/
|
||||
export function isReindexButtonLoading(args: {
|
||||
mutationPending: boolean;
|
||||
deadline: number | null;
|
||||
status?: boolean;
|
||||
}): boolean {
|
||||
const { mutationPending, deadline, status } = args;
|
||||
return mutationPending || (deadline !== null && status === true);
|
||||
}
|
||||
|
||||
// Translate the dot's tooltip label. Kept in one place so all three endpoint
|
||||
// cards share identical wording.
|
||||
function cardStatusLabel(status: CardStatus, t: (k: string) => string): string {
|
||||
@@ -215,31 +283,34 @@ export default function AiProviderSettings() {
|
||||
// PRE-job counts immediately, so the only way the "Indexed X of Y" counter
|
||||
// visibly climbs is to keep polling the settings query while the job runs.
|
||||
// `reindexDeadline` is the timestamp until which we poll (set on reindex
|
||||
// success); polling stops early once indexed === total. Bounded so a stuck
|
||||
// job can never poll forever.
|
||||
const REINDEX_POLL_INTERVAL = 3000; // ms between refetches while indexing
|
||||
// success). Polling tracks the server's `reindexing` flag: it keeps going for
|
||||
// the whole active run and stops promptly once the server reports the run is
|
||||
// finished. Bounded by the cap so a stuck/never-clearing progress record can
|
||||
// never poll forever.
|
||||
const REINDEX_POLL_INTERVAL = 5000; // ms between refetches while indexing
|
||||
const REINDEX_POLL_CAP_MS = 120000; // ~2 min hard cap
|
||||
const [reindexDeadline, setReindexDeadline] = useState<number | null>(null);
|
||||
|
||||
// Only admins may read the (masked) AI settings; the server enforces this too.
|
||||
const { data: settings, isLoading } = useAiSettingsQuery(isAdmin, (query) => {
|
||||
if (reindexDeadline === null) return false;
|
||||
// Past the cap → stop polling (cleared via the effect below too).
|
||||
if (Date.now() > reindexDeadline) return false;
|
||||
const data = query.state.data;
|
||||
// Stop once everything is indexed; otherwise keep polling.
|
||||
if (data && data.indexedPages >= data.totalPages) return false;
|
||||
return REINDEX_POLL_INTERVAL;
|
||||
});
|
||||
const { data: settings, isLoading } = useAiSettingsQuery(isAdmin, (query) =>
|
||||
nextReindexPollInterval({
|
||||
deadline: reindexDeadline,
|
||||
now: Date.now(),
|
||||
intervalMs: REINDEX_POLL_INTERVAL,
|
||||
status: query.state.data,
|
||||
}),
|
||||
);
|
||||
|
||||
// Stop polling once the work is done or the cap is reached. Also clears on
|
||||
// Stop polling once the run is finished or the cap is reached. Also clears on
|
||||
// unmount because the deadline state goes away with the component.
|
||||
useEffect(() => {
|
||||
if (reindexDeadline === null) return;
|
||||
// "Done" matches the refetchInterval stop condition (indexed >= total),
|
||||
// including an empty workspace (0 >= 0), so the deadline clears promptly
|
||||
// instead of waiting out the cap.
|
||||
if (settings && settings.indexedPages >= settings.totalPages) {
|
||||
// "Done" matches the refetchInterval stop condition: the server reports no
|
||||
// active run AND the count is complete (indexed >= total, incl. an empty
|
||||
// workspace 0 >= 0), so the deadline clears promptly instead of waiting out
|
||||
// the cap. While `reindexing` is still true we keep the deadline so polling
|
||||
// continues for the whole run.
|
||||
if (isReindexComplete(settings)) {
|
||||
setReindexDeadline(null);
|
||||
return;
|
||||
}
|
||||
@@ -1031,7 +1102,17 @@ export default function AiProviderSettings() {
|
||||
<Button
|
||||
variant="subtle"
|
||||
size="compact-sm"
|
||||
loading={reindexMutation.isPending}
|
||||
// Spin for the WHOLE run: the POST resolves immediately, but the
|
||||
// background job keeps running, so also stay loading while the
|
||||
// server reports `reindexing` (this also blocks a redundant
|
||||
// re-trigger mid-run; the server de-dupes regardless). The
|
||||
// deadline gate (and why it matters post-cap) lives in
|
||||
// `isReindexButtonLoading`, which is unit-tested.
|
||||
loading={isReindexButtonLoading({
|
||||
mutationPending: reindexMutation.isPending,
|
||||
deadline: reindexDeadline,
|
||||
status: settings?.reindexing,
|
||||
})}
|
||||
onClick={() =>
|
||||
reindexMutation.mutate(undefined, {
|
||||
// Begin bounded polling so the counter climbs as the async
|
||||
|
||||
@@ -23,8 +23,12 @@ export function useAiSettingsQuery(
|
||||
enabled: boolean = true,
|
||||
// While reindexing runs as an async background job, the counter only climbs
|
||||
// if the client keeps refetching. The component passes a refetchInterval
|
||||
// function that polls until indexed === total or a bounded deadline, then
|
||||
// returns false to stop. See AiProviderSettings.
|
||||
// function (`nextReindexPollInterval`) that keeps polling while the server
|
||||
// reports an active run (reindexing === true) OR we are still within the
|
||||
// bounded deadline and not yet fully indexed; it returns false to stop only
|
||||
// once the run has finished AND indexed >= total, or the deadline cap is hit
|
||||
// (the cap always wins). Note: a transient indexed === total during an active
|
||||
// run does NOT stop polling. See AiProviderSettings.
|
||||
refetchInterval?:
|
||||
| number
|
||||
| false
|
||||
|
||||
@@ -48,6 +48,9 @@ export interface IAiSettings {
|
||||
// RAG indexing coverage (pages indexed for semantic search).
|
||||
indexedPages: number;
|
||||
totalPages: number;
|
||||
// True while a full workspace reindex is actively running; the counts above
|
||||
// then reflect the live run progress (done climbs 0 -> total).
|
||||
reindexing?: boolean;
|
||||
}
|
||||
|
||||
// Update payload. Key semantics (same for `apiKey` and `embeddingApiKey`):
|
||||
|
||||
@@ -33,6 +33,11 @@ export class CollaborationGateway {
|
||||
// @ts-ignore
|
||||
private readonly redisSync: RedisSyncExtension<CollabEventHandlers> | null =
|
||||
null;
|
||||
// Source ioredis client that RedisSyncExtension duplicates into its pub/sub
|
||||
// pair. The extension's onDestroy only disconnects those duplicates, so we
|
||||
// keep a reference here and disconnect the source ourselves on shutdown
|
||||
// (otherwise the socket leaks and jest never exits in e2e).
|
||||
private redisClient: RedisClient | null = null;
|
||||
private readonly withRedis: boolean;
|
||||
|
||||
constructor(
|
||||
@@ -57,16 +62,17 @@ export class CollaborationGateway {
|
||||
});
|
||||
|
||||
if (this.withRedis) {
|
||||
this.redisClient = new RedisClient({
|
||||
host: this.redisConfig.host,
|
||||
port: this.redisConfig.port,
|
||||
password: this.redisConfig.password,
|
||||
db: this.redisConfig.db,
|
||||
family: this.redisConfig.family,
|
||||
retryStrategy: createRetryStrategy(),
|
||||
});
|
||||
// @ts-ignore
|
||||
this.redisSync = new RedisSyncExtension({
|
||||
redis: new RedisClient({
|
||||
host: this.redisConfig.host,
|
||||
port: this.redisConfig.port,
|
||||
password: this.redisConfig.password,
|
||||
db: this.redisConfig.db,
|
||||
family: this.redisConfig.family,
|
||||
retryStrategy: createRetryStrategy(),
|
||||
}),
|
||||
redis: this.redisClient,
|
||||
serverId: `collab-${os?.hostname()}-${nanoid(10)}`,
|
||||
prefix: 'collab',
|
||||
pack,
|
||||
@@ -184,5 +190,10 @@ export class CollaborationGateway {
|
||||
});
|
||||
|
||||
await this.hocuspocus.hooks('onDestroy', { instance: this.hocuspocus });
|
||||
|
||||
// RedisSyncExtension.onDestroy (run via the hook above) disconnects only the
|
||||
// duplicated pub/sub clients; the source client created here is ours to close.
|
||||
this.redisClient?.disconnect();
|
||||
this.redisClient = null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -36,6 +36,7 @@ import {
|
||||
Mention,
|
||||
Subpages,
|
||||
Highlight,
|
||||
Spoiler,
|
||||
Indent,
|
||||
UniqueID,
|
||||
Columns,
|
||||
@@ -82,6 +83,7 @@ export const tiptapExtensions = [
|
||||
Superscript,
|
||||
SubScript,
|
||||
Highlight,
|
||||
Spoiler,
|
||||
Typography,
|
||||
TrailingNode,
|
||||
TextStyle,
|
||||
|
||||
@@ -205,31 +205,203 @@ describe('PersistenceExtension.onStoreDocument — Approach-A boundary snapshot'
|
||||
expect(historyQueue.add).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// #206 persist-6 — RED (it.failing): a momentarily-empty live Y.Doc must not
|
||||
// overwrite non-empty persisted content. `onStoreDocument` empty-guards the
|
||||
// LOAD path but not the STORE path, so today an empty doc (a client/agent
|
||||
// glitch, a bad merge, an emptying transclusion) is written straight over the
|
||||
// page and the content is wiped silently. A store-side empty-guard is a real
|
||||
// behaviour change (a deliberate "select-all + delete" is also empty), so it
|
||||
// is left UNFIXED pending a product decision; this documents the data-loss
|
||||
// path and flips to a normal passing test the moment the guard lands.
|
||||
it.failing(
|
||||
'does NOT overwrite non-empty content with a momentarily-empty live doc (persist-6)',
|
||||
async () => {
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
// #206 persist-6 / #248 — a momentarily-empty live Y.Doc must not overwrite
|
||||
// non-empty persisted content. The store-side empty-guard blocks an empty doc
|
||||
// (a client/agent glitch, a bad merge, an emptying transclusion) from wiping
|
||||
// the page silently when NO intentional-clear signal is present.
|
||||
it('does NOT overwrite non-empty content with a momentarily-empty live doc (persist-6)', async () => {
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
|
||||
// Desired contract: the empty incoming doc is rejected and the rich page
|
||||
// survives. Today updatePage is called with the empty content (data loss).
|
||||
expect(pageRepo.updatePage).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
// The empty incoming doc is rejected and the rich page survives.
|
||||
expect(pageRepo.updatePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// #248 — an empty-over-empty store is allowed (nothing to lose); the guard
|
||||
// only protects non-empty persisted content.
|
||||
it('allows an empty store over already-empty content (#248)', async () => {
|
||||
const liveEmptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(liveEmptyDoc);
|
||||
// Stored content is empty per isEmptyParagraphDoc (paragraph with content:[])
|
||||
// but NOT deep-equal to the normalized live doc, so the unchanged
|
||||
// short-circuit is skipped and the empty-guard is genuinely reached.
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: { type: 'doc', content: [{ type: 'paragraph', content: [] }] },
|
||||
});
|
||||
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
|
||||
expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// #251 — REAL-PATH regression test. The intentional-clear signal is set via
|
||||
// the actual transport seam (ext.onStateless with the exact stateless payload
|
||||
// the client's IntentionalClear extension sends), NOT a hand-injected
|
||||
// context.intentionalClear poke. We then run the debounced store with an empty
|
||||
// live doc over non-empty persisted content and assert the empty write goes
|
||||
// through — i.e. the clear persists.
|
||||
it('persists an intentional clear signalled via the real stateless transport (#251)', async () => {
|
||||
const documentName = `page.${PAGE_ID}`;
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
|
||||
// The client signalled a deliberate clear over the live connection.
|
||||
await ext.onStateless({
|
||||
connection: { readOnly: false } as any,
|
||||
documentName,
|
||||
document: document as any,
|
||||
payload: JSON.stringify({ type: 'intentional-clear' }),
|
||||
} as any);
|
||||
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
|
||||
// The empty doc was written (the clear persisted). The persisted content is
|
||||
// the Y.Doc round-trip of the empty doc (attrs normalized), so compare
|
||||
// against fromYdoc rather than the raw literal.
|
||||
expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
|
||||
const expectedEmpty = TiptapTransformer.fromYdoc(document, 'default');
|
||||
expect(pageRepo.updatePage.mock.calls[0][0].content).toEqual(expectedEmpty);
|
||||
});
|
||||
|
||||
// #251 — retry correctness: a transient DB failure on the FIRST attempt must
|
||||
// not silently drop the clear. The intentional-clear flag is consumed ONCE
|
||||
// before the retry loop, so when attempt 1's updatePage throws (tx rolls back,
|
||||
// but the in-memory flag delete cannot roll back) the retry on attempt 2 still
|
||||
// sees the clear as allowed and writes the empty doc. On the pre-fix code
|
||||
// (consumeIntentionalClear called INSIDE the loop) attempt 1 consumed the flag,
|
||||
// attempt 2 re-read it as absent and the empty-guard BLOCKED the write — so
|
||||
// updatePage would be called once and the clear would be lost. This test fails
|
||||
// on that ordering and passes after the hoist.
|
||||
it('persists an intentional clear even when the first store attempt fails transiently (#251)', async () => {
|
||||
const documentName = `page.${PAGE_ID}`;
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
// The page stays non-empty in the DB across both attempts (the rolled-back
|
||||
// first attempt never changed it), exactly the failure scenario the WARNING
|
||||
// describes.
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
|
||||
let attempts = 0;
|
||||
pageRepo.updatePage.mockImplementation(async () => {
|
||||
attempts += 1;
|
||||
if (attempts === 1) throw new Error('deadlock detected'); // transient
|
||||
callOrder.push('updatePage');
|
||||
});
|
||||
|
||||
// The client signalled a deliberate clear over the live connection.
|
||||
await ext.onStateless({
|
||||
connection: { readOnly: false } as any,
|
||||
documentName,
|
||||
document: document as any,
|
||||
payload: JSON.stringify({ type: 'intentional-clear' }),
|
||||
} as any);
|
||||
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
|
||||
// First attempt failed and rolled back; the retry still honoured the clear
|
||||
// and wrote the empty doc (the clear survived the retry).
|
||||
expect(pageRepo.updatePage).toHaveBeenCalledTimes(2);
|
||||
const expectedEmpty = TiptapTransformer.fromYdoc(document, 'default');
|
||||
expect(pageRepo.updatePage.mock.calls[1][0].content).toEqual(expectedEmpty);
|
||||
});
|
||||
|
||||
// #251 — the signal is single-use: it is consumed by the first empty store,
|
||||
// so a SECOND accidental empty (no fresh signal) is still blocked.
|
||||
it('consumes the intentional-clear signal once; a later empty is blocked (#251)', async () => {
|
||||
const documentName = `page.${PAGE_ID}`;
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
|
||||
await ext.onStateless({
|
||||
connection: { readOnly: false } as any,
|
||||
documentName,
|
||||
document: ydocFor(emptyDoc) as any,
|
||||
payload: JSON.stringify({ type: 'intentional-clear' }),
|
||||
} as any);
|
||||
|
||||
// First empty store consumes the signal and writes.
|
||||
await ext.onStoreDocument(buildData(ydocFor(emptyDoc), 'user') as any);
|
||||
expect(pageRepo.updatePage).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Re-arm findById to non-empty (as if content came back) and fire another
|
||||
// empty store WITHOUT a new signal — the guard must block it.
|
||||
pageRepo.updatePage.mockClear();
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
await ext.onStoreDocument(buildData(ydocFor(emptyDoc), 'user') as any);
|
||||
expect(pageRepo.updatePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// #251 — a read-only connection cannot arm the clear, so its empty store is
|
||||
// still blocked (defends the guard against a read-only spoof).
|
||||
it('ignores an intentional-clear signal from a read-only connection (#251)', async () => {
|
||||
const documentName = `page.${PAGE_ID}`;
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
const document = ydocFor(emptyDoc);
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
|
||||
await ext.onStateless({
|
||||
connection: { readOnly: true } as any,
|
||||
documentName,
|
||||
document: document as any,
|
||||
payload: JSON.stringify({ type: 'intentional-clear' }),
|
||||
} as any);
|
||||
|
||||
await ext.onStoreDocument(buildData(document, 'user') as any);
|
||||
|
||||
expect(pageRepo.updatePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// #251 — a non-empty store between the signal and the empty store drops the
|
||||
// pending flag ("cleared then retyped" can't leave a usable signal behind).
|
||||
it('drops a pending clear when a non-empty store intervenes (#251)', async () => {
|
||||
const documentName = `page.${PAGE_ID}`;
|
||||
const emptyDoc = { type: 'doc', content: [{ type: 'paragraph' }] };
|
||||
|
||||
await ext.onStateless({
|
||||
connection: { readOnly: false } as any,
|
||||
documentName,
|
||||
document: ydocFor(emptyDoc) as any,
|
||||
payload: JSON.stringify({ type: 'intentional-clear' }),
|
||||
} as any);
|
||||
|
||||
// A non-empty store lands first → consumes/drops the stale flag.
|
||||
pageRepo.findById.mockResolvedValue(persistedHumanPage('NEW HUMAN TEXT'));
|
||||
await ext.onStoreDocument(
|
||||
buildData(ydocFor(doc('NEW HUMAN TEXT')), 'user') as any,
|
||||
);
|
||||
pageRepo.updatePage.mockClear();
|
||||
|
||||
// Now an empty store with no fresh signal must be blocked.
|
||||
pageRepo.findById.mockResolvedValue({
|
||||
...persistedHumanPage('IGNORED'),
|
||||
content: doc('IMPORTANT RICH CONTENT'),
|
||||
});
|
||||
await ext.onStoreDocument(buildData(ydocFor(emptyDoc), 'user') as any);
|
||||
expect(pageRepo.updatePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// persist-1 — when every attempt fails the hook must NOT report a phantom
|
||||
// success: no "page.updated" badge broadcast and no history snapshot for
|
||||
|
||||
@@ -3,6 +3,7 @@ import {
|
||||
Extension,
|
||||
onChangePayload,
|
||||
onLoadDocumentPayload,
|
||||
onStatelessPayload,
|
||||
onStoreDocumentPayload,
|
||||
} from '@hocuspocus/server';
|
||||
import * as Y from 'yjs';
|
||||
@@ -41,6 +42,35 @@ import {
|
||||
} from '../constants';
|
||||
import { TransclusionService } from '../../core/page/transclusion/transclusion.service';
|
||||
|
||||
/**
|
||||
* #251 — wire format of the client→server stateless message that signals a
|
||||
* deliberate page clear. The client (IntentionalClear editor extension) sends
|
||||
* `{ type: INTENTIONAL_CLEAR_MESSAGE_TYPE }`; the document is taken from the
|
||||
* connection, not the payload, so the signal cannot be aimed at another page.
|
||||
*/
|
||||
export const INTENTIONAL_CLEAR_MESSAGE_TYPE = 'intentional-clear';
|
||||
|
||||
/**
|
||||
* #251 — how long an intentional-clear signal stays "pending" before it is
|
||||
* ignored. The signal is set on the clearing keystroke but consumed by the
|
||||
* DEBOUNCED onStoreDocument, so the TTL must comfortably exceed the collab
|
||||
* store debounce window (hocuspocus is configured with maxDebounce = 45s in
|
||||
* collaboration.gateway.ts). 60s leaves a margin while keeping the window for a
|
||||
* stale flag small; on top of the TTL, any non-empty store immediately drops a
|
||||
* pending flag (see onStoreDocument), so a "cleared then retyped" sequence can
|
||||
* never leave a usable flag behind.
|
||||
*
|
||||
* Known fail-safe limitation: the flag lives only in this node's process memory.
|
||||
* If document ownership transfers to another node, or this node crashes/restarts,
|
||||
* between the stateless signal (set on node A) and the debounced store, the
|
||||
* in-memory flag is lost and the clear is silently NOT applied — the store-side
|
||||
* empty-guard then reloads the document non-empty from the DB. This is
|
||||
* deliberately fail-safe (a lost flag preserves content rather than destroying
|
||||
* it), but it is a documented limitation, not a guarantee that every deliberate
|
||||
* clear survives a node handoff.
|
||||
*/
|
||||
export const INTENTIONAL_CLEAR_TTL_MS = 60_000;
|
||||
|
||||
/**
|
||||
* Resolve the provenance source for a coalesced snapshot.
|
||||
*
|
||||
@@ -96,6 +126,13 @@ export class PersistenceExtension implements Extension {
|
||||
// coalescing window" per document and OR it across all edits in the window,
|
||||
// so the snapshot is marked 'agent' regardless of who wrote last.
|
||||
private agentTouched: Map<string, boolean> = new Map();
|
||||
// #251 — per-document "intentional clear pending" flags. Keyed by
|
||||
// documentName, value = expiry timestamp (ms). Set by onStateless when the
|
||||
// client reports a deliberate clear; consumed once by the next
|
||||
// onStoreDocument empty-guard branch. This is the per-EDIT channel the
|
||||
// per-connection context cannot provide (a clear is an edit event, but the
|
||||
// store is debounced and connection context is fixed at authentication).
|
||||
private intentionalClear: Map<string, number> = new Map();
|
||||
|
||||
constructor(
|
||||
private readonly pageRepo: PageRepo,
|
||||
@@ -180,6 +217,19 @@ export class PersistenceExtension implements Extension {
|
||||
this.consumeAgentTouched(documentName),
|
||||
context?.actor,
|
||||
);
|
||||
// #251 — consume the intentional-clear flag ONCE, BEFORE the retry loop
|
||||
// (like consumeContributors / consumeAgentTouched above). consumeIntentional-
|
||||
// Clear ALWAYS deletes the in-memory Map entry, but a tx rollback cannot
|
||||
// un-delete it. Calling it INSIDE the loop meant: a clear armed for attempt 1
|
||||
// was consumed there, attempt 1's updatePage threw a transient error and
|
||||
// rolled back, then attempt 2 re-read non-empty content and saw the flag
|
||||
// already gone — silently downgrading the retry into a BLOCKED write, so the
|
||||
// user's deliberate clear was dropped. Hoisting makes the decision stable
|
||||
// across every attempt. This single call also preserves the "a non-empty
|
||||
// store drops a pending flag" semantics (the cleared-then-retyped case):
|
||||
// every store consumes the flag here regardless of incoming emptiness, so a
|
||||
// subsequent non-empty store can never leave a usable flag behind.
|
||||
const allowIntentionalClear = this.consumeIntentionalClear(documentName);
|
||||
|
||||
// Persist with a small bounded retry. The in-memory Y.Doc is the ONLY copy
|
||||
// of the latest edit until this hook returns: hocuspocus destroys/unloads the
|
||||
@@ -210,6 +260,46 @@ export class PersistenceExtension implements Extension {
|
||||
return;
|
||||
}
|
||||
|
||||
// #206 persist-6 / #248 — store-side empty-guard. A momentarily-empty
|
||||
// live Y.Doc (a client/agent glitch, a bad merge, a transclusion that
|
||||
// emptied) must NOT overwrite non-empty persisted content. The LOAD
|
||||
// path already guards emptiness (onLoadDocument only hydrates from db
|
||||
// when the live doc isEmpty); the STORE path did not, so an empty
|
||||
// serialization was written straight over the page, wiping it
|
||||
// silently.
|
||||
//
|
||||
// #251 — the ONE legitimate empty-over-non-empty write is a user who
|
||||
// deliberately clears the page. That intent arrives out-of-band as a
|
||||
// stateless message, NOT from the doc content, which is why it cannot
|
||||
// be spoofed for non-clear writes: the flag is only ever read on this
|
||||
// empty-incoming branch, so the worst a forged signal can do is clear
|
||||
// a page the connection may already edit. The flag was consumed ONCE
|
||||
// before the retry loop (`allowIntentionalClear`) so the decision is
|
||||
// stable across retries; a non-empty store still drops any pending
|
||||
// flag via that same hoisted consume (a "cleared then retyped"
|
||||
// sequence can't leave a usable one behind).
|
||||
const incomingEmpty = isEmptyParagraphDoc(tiptapJson as any);
|
||||
if (
|
||||
incomingEmpty &&
|
||||
page.content &&
|
||||
!isEmptyParagraphDoc(page.content as any)
|
||||
) {
|
||||
if (allowIntentionalClear) {
|
||||
this.logger.debug(
|
||||
`Intentional clear for ${pageId}: persisting empty doc over ` +
|
||||
`non-empty content (user-signalled)`,
|
||||
);
|
||||
// fall through — the empty write is allowed exactly once.
|
||||
} else {
|
||||
this.logger.warn(
|
||||
`Skipping store for ${pageId}: empty live doc would overwrite ` +
|
||||
`non-empty persisted content`,
|
||||
);
|
||||
page = null;
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
let contributorIds = undefined;
|
||||
try {
|
||||
const existingContributors = page.contributorIds || [];
|
||||
@@ -345,6 +435,37 @@ export class PersistenceExtension implements Extension {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* #251 — receive the client's deliberate-clear signal. Records a short-lived,
|
||||
* single-use pending flag for the originating document so the next
|
||||
* onStoreDocument may let one empty-over-non-empty write through the guard.
|
||||
*
|
||||
* Hardening: read-only connections cannot arm the flag, and the document is
|
||||
* taken from the connection (`data.documentName`), never the payload, so a
|
||||
* client cannot target a page it isn't editing. The flag only ever RELAXES
|
||||
* the guard for an empty write (a clear); it can never force or alter a
|
||||
* non-empty write, so it is not a guard bypass for normal content.
|
||||
*/
|
||||
async onStateless(data: onStatelessPayload) {
|
||||
const { connection, documentName, payload } = data;
|
||||
|
||||
if (connection?.readOnly) return;
|
||||
|
||||
let message: { type?: string } | undefined;
|
||||
try {
|
||||
message = JSON.parse(payload);
|
||||
} catch {
|
||||
return; // unrelated / malformed stateless message
|
||||
}
|
||||
|
||||
if (message?.type !== INTENTIONAL_CLEAR_MESSAGE_TYPE) return;
|
||||
|
||||
this.intentionalClear.set(
|
||||
documentName,
|
||||
Date.now() + INTENTIONAL_CLEAR_TTL_MS,
|
||||
);
|
||||
}
|
||||
|
||||
async onChange(data: onChangePayload) {
|
||||
const documentName = data.documentName;
|
||||
const userId = data.context?.user?.id;
|
||||
@@ -368,6 +489,7 @@ export class PersistenceExtension implements Extension {
|
||||
const documentName = data.documentName;
|
||||
this.contributors.delete(documentName);
|
||||
this.agentTouched.delete(documentName);
|
||||
this.intentionalClear.delete(documentName);
|
||||
}
|
||||
|
||||
private consumeContributors(documentName: string): string[] {
|
||||
@@ -385,6 +507,18 @@ export class PersistenceExtension implements Extension {
|
||||
return touched;
|
||||
}
|
||||
|
||||
/**
|
||||
* #251 — read and clear the intentional-clear flag for this document. Returns
|
||||
* true only if a flag was pending AND still within its TTL. Always deletes the
|
||||
* entry so the signal is strictly single-use (one clear → one allowed empty
|
||||
* write); an expired flag is treated as absent (guard still blocks).
|
||||
*/
|
||||
private consumeIntentionalClear(documentName: string): boolean {
|
||||
const expiry = this.intentionalClear.get(documentName);
|
||||
this.intentionalClear.delete(documentName);
|
||||
return expiry !== undefined && Date.now() < expiry;
|
||||
}
|
||||
|
||||
private async enqueuePageHistory(
|
||||
page: Page,
|
||||
lastUpdatedSource: string,
|
||||
|
||||
@@ -3,6 +3,8 @@ import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
||||
import { PageEmbeddingRepo } from '@docmost/db/repos/ai-chat/page-embedding.repo';
|
||||
import { KyselyDB } from '@docmost/db/types/kysely.types';
|
||||
import { AiService } from '../../../integrations/ai/ai.service';
|
||||
import { EmbeddingReindexProgressService } from '../../../integrations/ai/embedding-reindex-progress.service';
|
||||
import { AiEmbeddingNotConfiguredException } from '../../../integrations/ai/ai-embedding-not-configured.exception';
|
||||
|
||||
/**
|
||||
* Unit tests for EmbeddingIndexerService.reindexWorkspace's batch control flow.
|
||||
@@ -12,7 +14,8 @@ import { AiService } from '../../../integrations/ai/ai.service';
|
||||
* reindexWorkspace actually touches:
|
||||
* - aiService.getEmbeddingModel -> a model string so the up-front configured
|
||||
* check passes,
|
||||
* - pageRepo.getIdsByWorkspace -> three page ids,
|
||||
* - pageRepo.getEmbeddablePageIds -> three page ids (the embeddable set the
|
||||
* reindex iterates),
|
||||
* - service.reindexPage -> spied per test to drive the per-page outcome.
|
||||
*
|
||||
* The point under test is the catch block: a FATAL provider error (auth/billing)
|
||||
@@ -24,21 +27,30 @@ describe('EmbeddingIndexerService.reindexWorkspace fail-fast', () => {
|
||||
|
||||
function makeService() {
|
||||
const pageRepo = {
|
||||
getIdsByWorkspace: jest.fn().mockResolvedValue(['p1', 'p2', 'p3']),
|
||||
getEmbeddablePageIds: jest.fn().mockResolvedValue(['p1', 'p2', 'p3']),
|
||||
};
|
||||
const pageEmbeddingRepo = {};
|
||||
const aiService = {
|
||||
getEmbeddingModel: jest.fn().mockResolvedValue('some-model'),
|
||||
};
|
||||
// Progress is a best-effort cosmetic store; mock its async methods so the
|
||||
// batch control flow can be tested without Redis.
|
||||
const reindexProgress = {
|
||||
start: jest.fn().mockResolvedValue(undefined),
|
||||
increment: jest.fn().mockResolvedValue(undefined),
|
||||
clear: jest.fn().mockResolvedValue(undefined),
|
||||
get: jest.fn().mockResolvedValue(null),
|
||||
};
|
||||
const db = {};
|
||||
|
||||
const service = new EmbeddingIndexerService(
|
||||
pageRepo as unknown as PageRepo,
|
||||
pageEmbeddingRepo as unknown as PageEmbeddingRepo,
|
||||
aiService as unknown as AiService,
|
||||
reindexProgress as unknown as EmbeddingReindexProgressService,
|
||||
db as unknown as KyselyDB,
|
||||
);
|
||||
return { service, pageRepo, aiService };
|
||||
return { service, pageRepo, aiService, reindexProgress };
|
||||
}
|
||||
|
||||
it('aborts after the first page on a FATAL (401) provider error', async () => {
|
||||
@@ -78,3 +90,100 @@ describe('EmbeddingIndexerService.reindexWorkspace fail-fast', () => {
|
||||
expect(reindexPage).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Live reindex-progress reporting: reindexWorkspace must publish a per-workspace
|
||||
* progress record (total at start, done incremented per processed page) and ALWAYS
|
||||
* clear it in a finally — including on a fatal abort and an unconfigured early
|
||||
* return — so the settings status can show the counter climb without ever getting
|
||||
* stuck in a "reindexing" state.
|
||||
*/
|
||||
describe('EmbeddingIndexerService.reindexWorkspace progress', () => {
|
||||
const WORKSPACE_ID = 'ws-1';
|
||||
|
||||
function makeService(pageIds: string[] = ['p1', 'p2', 'p3']) {
|
||||
const pageRepo = {
|
||||
getEmbeddablePageIds: jest.fn().mockResolvedValue(pageIds),
|
||||
};
|
||||
const pageEmbeddingRepo = {};
|
||||
const aiService = {
|
||||
getEmbeddingModel: jest.fn().mockResolvedValue('some-model'),
|
||||
};
|
||||
const reindexProgress = {
|
||||
start: jest.fn().mockResolvedValue(undefined),
|
||||
increment: jest.fn().mockResolvedValue(undefined),
|
||||
clear: jest.fn().mockResolvedValue(undefined),
|
||||
get: jest.fn().mockResolvedValue(null),
|
||||
};
|
||||
const db = {};
|
||||
const service = new EmbeddingIndexerService(
|
||||
pageRepo as unknown as PageRepo,
|
||||
pageEmbeddingRepo as unknown as PageEmbeddingRepo,
|
||||
aiService as unknown as AiService,
|
||||
reindexProgress as unknown as EmbeddingReindexProgressService,
|
||||
db as unknown as KyselyDB,
|
||||
);
|
||||
return { service, pageRepo, aiService, reindexProgress };
|
||||
}
|
||||
|
||||
it('sets total at start, increments done per page, and clears in finally', async () => {
|
||||
const { service, reindexProgress } = makeService(['p1', 'p2', 'p3']);
|
||||
jest.spyOn(service, 'reindexPage').mockResolvedValue(undefined);
|
||||
|
||||
await service.reindexWorkspace(WORKSPACE_ID);
|
||||
|
||||
expect(reindexProgress.start).toHaveBeenCalledWith(WORKSPACE_ID, 3);
|
||||
// One increment per processed page.
|
||||
expect(reindexProgress.increment).toHaveBeenCalledTimes(3);
|
||||
expect(reindexProgress.increment).toHaveBeenCalledWith(WORKSPACE_ID);
|
||||
// Cleared exactly once on completion.
|
||||
expect(reindexProgress.clear).toHaveBeenCalledTimes(1);
|
||||
expect(reindexProgress.clear).toHaveBeenCalledWith(WORKSPACE_ID);
|
||||
});
|
||||
|
||||
it('counts a handled (non-fatal) per-page failure as processed', async () => {
|
||||
const { service, reindexProgress } = makeService(['p1', 'p2', 'p3']);
|
||||
// No statusCode -> non-fatal -> isolate and continue; each counts as done.
|
||||
jest.spyOn(service, 'reindexPage').mockRejectedValue(new Error('boom'));
|
||||
|
||||
await service.reindexWorkspace(WORKSPACE_ID);
|
||||
|
||||
expect(reindexProgress.increment).toHaveBeenCalledTimes(3);
|
||||
expect(reindexProgress.clear).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('clears progress in finally even when a FATAL provider error aborts the batch', async () => {
|
||||
const { service, reindexProgress } = makeService(['p1', 'p2', 'p3']);
|
||||
// A 401 aborts on the first page (re-thrown) — the finally must still clear.
|
||||
jest
|
||||
.spyOn(service, 'reindexPage')
|
||||
.mockRejectedValue({ statusCode: 401, message: 'User not found' });
|
||||
|
||||
await expect(service.reindexWorkspace(WORKSPACE_ID)).rejects.toMatchObject({
|
||||
statusCode: 401,
|
||||
});
|
||||
|
||||
expect(reindexProgress.start).toHaveBeenCalledWith(WORKSPACE_ID, 3);
|
||||
// Aborted page is NOT counted as processed.
|
||||
expect(reindexProgress.increment).not.toHaveBeenCalled();
|
||||
// But progress is still cleared so the run never gets stuck.
|
||||
expect(reindexProgress.clear).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('clears the enqueue-seeded progress on an unconfigured early return', async () => {
|
||||
const { service, aiService, reindexProgress } = makeService();
|
||||
// Embeddings not configured: reindexWorkspace returns early WITHOUT starting
|
||||
// a fresh record, but the finally must still clear the enqueue-time seed.
|
||||
aiService.getEmbeddingModel = jest
|
||||
.fn()
|
||||
.mockRejectedValue(new AiEmbeddingNotConfiguredException());
|
||||
|
||||
await expect(
|
||||
service.reindexWorkspace(WORKSPACE_ID),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(reindexProgress.start).not.toHaveBeenCalled();
|
||||
expect(reindexProgress.clear).toHaveBeenCalledTimes(1);
|
||||
expect(reindexProgress.clear).toHaveBeenCalledWith(WORKSPACE_ID);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,6 +9,7 @@ import { KyselyDB } from '@docmost/db/types/kysely.types';
|
||||
import { InjectKysely } from 'nestjs-kysely';
|
||||
import { executeTx } from '@docmost/db/utils';
|
||||
import { AiService } from '../../../integrations/ai/ai.service';
|
||||
import { EmbeddingReindexProgressService } from '../../../integrations/ai/embedding-reindex-progress.service';
|
||||
import { AiEmbeddingNotConfiguredException } from '../../../integrations/ai/ai-embedding-not-configured.exception';
|
||||
import {
|
||||
describeProviderError,
|
||||
@@ -48,6 +49,7 @@ export class EmbeddingIndexerService {
|
||||
private readonly pageRepo: PageRepo,
|
||||
private readonly pageEmbeddingRepo: PageEmbeddingRepo,
|
||||
private readonly aiService: AiService,
|
||||
private readonly reindexProgress: EmbeddingReindexProgressService,
|
||||
@InjectKysely() private readonly db: KyselyDB,
|
||||
) {}
|
||||
|
||||
@@ -183,7 +185,19 @@ export class EmbeddingIndexerService {
|
||||
}
|
||||
|
||||
/**
|
||||
* (Re)build embeddings for EVERY non-deleted page in a workspace. Used by the
|
||||
* (Re)build embeddings for the EMBEDDABLE page set of a workspace — the same
|
||||
* set countEmbeddablePages counts (via getEmbeddablePageIds): non-deleted pages
|
||||
* that qualify under any of the three clauses of `embeddablePredicate` —
|
||||
* non-empty textContent, OR an empty/null textContent whose ProseMirror
|
||||
* `content` JSON has at least one text node (`"type":"text"`) that `jsonToText`
|
||||
* can extract, OR an already-stored (non-deleted) embedding row — NOT every
|
||||
* non-deleted page. Iterating this set keeps the live `total` equal to the
|
||||
* steady-state denominator, so the progress counter climbs 0 -> total and
|
||||
* matches the before/after DB coverage exactly. A page with truly no
|
||||
* extractable text (empty textContent AND content with only non-text/atom
|
||||
* nodes such as math) is correctly skipped (reindexPage no-ops on it); a page
|
||||
* that lost its text but still has stale embeddings stays in the set (the
|
||||
* EXISTS clause) so it is visited and its stale rows are cleared. Used by the
|
||||
* bulk reindex (WORKSPACE_CREATE_EMBEDDINGS, fired when AI Search is enabled
|
||||
* and by the manual "Reindex now" action).
|
||||
*
|
||||
@@ -194,69 +208,99 @@ export class EmbeddingIndexerService {
|
||||
* the batch.
|
||||
*/
|
||||
async reindexWorkspace(workspaceId: string): Promise<void> {
|
||||
// The whole run is wrapped so the per-workspace progress record is ALWAYS
|
||||
// cleared in the finally — on success, on a fatal-provider abort, on an
|
||||
// unconfigured early-return, or on any unexpected throw — so a failed run
|
||||
// never leaves a stuck "reindexing" state (the status then falls back to the
|
||||
// steady-state DB coverage count). A placeholder record may already exist
|
||||
// (seeded at enqueue time); the finally cleans that too.
|
||||
try {
|
||||
await this.aiService.getEmbeddingModel(workspaceId);
|
||||
} catch (err) {
|
||||
if (err instanceof AiEmbeddingNotConfiguredException) {
|
||||
this.logger.log(
|
||||
`reindexWorkspace: embeddings not configured for workspace ${workspaceId}, skipping`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
const pageIds = await this.pageRepo.getIdsByWorkspace(workspaceId);
|
||||
const total = pageIds.length;
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(
|
||||
`reindexWorkspace: starting reindex of ${total} page(s) for workspace ${workspaceId}`,
|
||||
);
|
||||
|
||||
let failed = 0;
|
||||
for (let i = 0; i < total; i++) {
|
||||
const pageId = pageIds[i];
|
||||
const position = i + 1;
|
||||
// Log BEFORE the await: if the embedding call hangs, this is the last line
|
||||
// in the log and it names the exact page that is stuck.
|
||||
this.logger.log(
|
||||
`reindexWorkspace: [${position}/${total}] indexing page ${pageId} (workspace ${workspaceId})`,
|
||||
);
|
||||
const pageStartedAt = Date.now();
|
||||
try {
|
||||
await this.reindexPage(pageId);
|
||||
const elapsed = Date.now() - pageStartedAt;
|
||||
if (elapsed >= SLOW_PAGE_MS) {
|
||||
this.logger.warn(
|
||||
`reindexWorkspace: [${position}/${total}] page ${pageId} took ${elapsed}ms`,
|
||||
);
|
||||
}
|
||||
await this.aiService.getEmbeddingModel(workspaceId);
|
||||
} catch (err) {
|
||||
// A fatal provider error (invalid/missing key, no credits) recurs
|
||||
// identically on EVERY remaining page. Abort the whole batch instead of
|
||||
// issuing hundreds of doomed requests against the provider.
|
||||
if (isFatalProviderError(err)) {
|
||||
this.logger.error(
|
||||
`reindexWorkspace: aborting at [${position}/${total}] for workspace ` +
|
||||
`${workspaceId} — fatal provider error, remaining pages would fail ` +
|
||||
`identically: ${describeProviderError(err)}`,
|
||||
if (err instanceof AiEmbeddingNotConfiguredException) {
|
||||
this.logger.log(
|
||||
`reindexWorkspace: embeddings not configured for workspace ${workspaceId}, skipping`,
|
||||
);
|
||||
throw err;
|
||||
return;
|
||||
}
|
||||
// Per-page isolation: one non-fatal failure (incl. an embedding timeout)
|
||||
// must not abort the whole batch.
|
||||
failed++;
|
||||
this.logger.error(
|
||||
`reindexWorkspace: [${position}/${total}] failed to reindex page ${pageId} ` +
|
||||
`after ${Date.now() - pageStartedAt}ms: ${describeProviderError(err)}`,
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
this.logger.log(
|
||||
`reindexWorkspace: done for workspace ${workspaceId}: ` +
|
||||
`${total - failed}/${total} indexed, ${failed} failed in ${Date.now() - startedAt}ms`,
|
||||
);
|
||||
// Iterate the EMBEDDABLE set (same three-clause predicate as
|
||||
// countEmbeddablePages), NOT every non-deleted page: this makes `total`
|
||||
// here equal the steady-state denominator, so the live counter climbs
|
||||
// 0 -> total and matches the before/after DB count exactly (no
|
||||
// 478 -> 500 -> 478 denominator jump). Pages whose text lives in the
|
||||
// ProseMirror `content` JSON (a text node) even with empty text_content ARE
|
||||
// in this set (the content-JSON clause) and get embedded; a page with no
|
||||
// extractable text at all is correctly skipped — reindexPage no-ops on it —
|
||||
// and a page that lost its text but still has stale embeddings IS in this
|
||||
// set (the EXISTS clause) so it is still visited and its stale rows cleared.
|
||||
const pageIds = await this.pageRepo.getEmbeddablePageIds(workspaceId);
|
||||
const total = pageIds.length;
|
||||
const startedAt = Date.now();
|
||||
// Publish the live run progress over this same set (done reset to 0). The
|
||||
// counter increments once per iterated page and reaches exactly `total`,
|
||||
// which equals countEmbeddablePages — the steady-state denominator.
|
||||
await this.reindexProgress.start(workspaceId, total);
|
||||
this.logger.log(
|
||||
`reindexWorkspace: starting reindex of ${total} page(s) for workspace ${workspaceId}`,
|
||||
);
|
||||
|
||||
let failed = 0;
|
||||
for (let i = 0; i < total; i++) {
|
||||
const pageId = pageIds[i];
|
||||
const position = i + 1;
|
||||
// Log BEFORE the await: if the embedding call hangs, this is the last line
|
||||
// in the log and it names the exact page that is stuck.
|
||||
this.logger.log(
|
||||
`reindexWorkspace: [${position}/${total}] indexing page ${pageId} (workspace ${workspaceId})`,
|
||||
);
|
||||
const pageStartedAt = Date.now();
|
||||
try {
|
||||
await this.reindexPage(pageId);
|
||||
// Count this page as processed (matches the [position/total] log).
|
||||
await this.reindexProgress.increment(workspaceId);
|
||||
const elapsed = Date.now() - pageStartedAt;
|
||||
if (elapsed >= SLOW_PAGE_MS) {
|
||||
this.logger.warn(
|
||||
`reindexWorkspace: [${position}/${total}] page ${pageId} took ${elapsed}ms`,
|
||||
);
|
||||
}
|
||||
} catch (err) {
|
||||
// A fatal provider error (invalid/missing key, no credits) recurs
|
||||
// identically on EVERY remaining page. Abort the whole batch instead of
|
||||
// issuing hundreds of doomed requests against the provider. Do NOT count
|
||||
// it as processed — the run aborts here (the finally clears progress).
|
||||
if (isFatalProviderError(err)) {
|
||||
this.logger.error(
|
||||
`reindexWorkspace: aborting at [${position}/${total}] for workspace ` +
|
||||
`${workspaceId} — fatal provider error, remaining pages would fail ` +
|
||||
`identically: ${describeProviderError(err)}`,
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
// Per-page isolation: one non-fatal failure (incl. an embedding timeout)
|
||||
// must not abort the whole batch. A handled failure still advances the
|
||||
// counter (matches the [position/total] log, so done reaches total).
|
||||
failed++;
|
||||
await this.reindexProgress.increment(workspaceId);
|
||||
this.logger.error(
|
||||
`reindexWorkspace: [${position}/${total}] failed to reindex page ${pageId} ` +
|
||||
`after ${Date.now() - pageStartedAt}ms: ${describeProviderError(err)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
this.logger.log(
|
||||
`reindexWorkspace: done for workspace ${workspaceId}: ` +
|
||||
`${total - failed}/${total} indexed, ${failed} failed in ${Date.now() - startedAt}ms`,
|
||||
);
|
||||
} finally {
|
||||
// Always remove the progress record so the status reverts to the DB count.
|
||||
await this.reindexProgress.clear(workspaceId);
|
||||
}
|
||||
}
|
||||
|
||||
/** Purge ALL embeddings for a workspace (WORKSPACE_DELETE_EMBEDDINGS). */
|
||||
|
||||
@@ -5,6 +5,34 @@ import { pathToFileURL } from 'node:url';
|
||||
* ESM-only `@docmost/mcp` package. We only need the constructor + the read/write
|
||||
* methods used by the per-user tool adapter; the full client surface lives in
|
||||
* `packages/mcp/src/client.ts`. Signatures here mirror that file exactly.
|
||||
*
|
||||
* DRIFT GUARD: the method NAMES below are runtime-checked against the real
|
||||
* `DocmostClient` by `packages/mcp/test/unit/client-host-contract.test.mjs`
|
||||
* (which can import the ESM class directly). If you rename/remove a method here
|
||||
* or in client.ts, that test fails — so a stale mirror cannot silently ship a
|
||||
* runtime "x is not a function" into an agent tool call. Keep the two in sync.
|
||||
*
|
||||
* STAGED PLAN — full derivation `DocmostClientLike = <real DocmostClient type>`
|
||||
* (issue #193, layer 3) is intentionally NOT done; it stays a hand-mirror for
|
||||
* now because of two verified blockers across the ESM(mcp)/CJS(server) boundary:
|
||||
* 1. `@docmost/mcp` emits NO declaration files (its tsconfig has no
|
||||
* `declaration`, package.json has no `types`/types-export) and the server
|
||||
* tsconfig has no path mapping for it — the server only loads it via the
|
||||
* runtime `import()` trick below, so there is no type to import today.
|
||||
* 2. The real client methods have inferred, CONCRETE return types; the in-app
|
||||
* tool adapter reads results through loose `Record<string,unknown>` returns
|
||||
* + `as` casts (e.g. `(result?.data ?? {}) as { title?: string }`).
|
||||
* Deriving the exact type would make those casts non-overlapping ("may be a
|
||||
* mistake") and break the build, and `Partial<DocmostClientLike>` test stubs
|
||||
* would have to satisfy the full concrete surface.
|
||||
* To do it safely later (incrementally): (a) turn on `declaration: true` in
|
||||
* packages/mcp/tsconfig.json + add a `types` export condition and commit the
|
||||
* emitted `.d.ts`; (b) `import type { DocmostClient } from '@docmost/mcp'` here
|
||||
* and replace this interface with a `Pick<DocmostClient, ...>` of the consumed
|
||||
* methods; (c) audit every `as` cast in ai-chat-tools.service.ts against the now
|
||||
* concrete return types (double-cast through `unknown` only where genuinely
|
||||
* needed); (d) keep the runtime guard test as a belt-and-braces check. Until
|
||||
* then the guard test above is the cheap, behaviour-neutral protection.
|
||||
*/
|
||||
export interface DocmostClientLike {
|
||||
// --- read ---
|
||||
|
||||
129
apps/server/src/core/share/share-spoiler-keep.spec.ts
Normal file
129
apps/server/src/core/share/share-spoiler-keep.spec.ts
Normal file
@@ -0,0 +1,129 @@
|
||||
import { ShareService } from './share.service';
|
||||
|
||||
// Sibling of share-comment-strip.spec.ts. The public-share sanitizer strips ONLY
|
||||
// `comment` marks (internal-team metadata) via removeMarkTypeFromDoc(doc,
|
||||
// 'comment'). The `spoiler` mark is legitimate authored content (hidden text the
|
||||
// reader clicks to reveal) and MUST survive the share-strip — otherwise public
|
||||
// readers would see the secret in plain text or lose it entirely.
|
||||
//
|
||||
// We drive the SAME real seam the comment-strip test uses:
|
||||
// updatePublicAttachments -> prepareContentForShare -> removeMarkTypeFromDoc.
|
||||
|
||||
const WS = 'ws-1';
|
||||
const PAGE = 'page-1';
|
||||
|
||||
function buildService() {
|
||||
const shareRepo = { findById: jest.fn() };
|
||||
const pageRepo = { findById: jest.fn() };
|
||||
const pagePermissionRepo = {
|
||||
hasRestrictedAncestor: jest.fn(async () => false),
|
||||
};
|
||||
const tokenService = {
|
||||
generateAttachmentToken: jest.fn(async () => 'tok'),
|
||||
};
|
||||
const workspaceRepo = {
|
||||
findById: jest.fn(async () => ({ id: WS, settings: { htmlEmbed: true } })),
|
||||
};
|
||||
|
||||
return new ShareService(
|
||||
shareRepo as any,
|
||||
pageRepo as any,
|
||||
pagePermissionRepo as any,
|
||||
{} as any, // db (unused on this path)
|
||||
tokenService as any,
|
||||
{} as any, // transclusionService (unused)
|
||||
workspaceRepo as any,
|
||||
);
|
||||
}
|
||||
|
||||
// Text carrying a `spoiler` mark (no attributes; revealed state is UI-only).
|
||||
function spoilerText(text: string) {
|
||||
return {
|
||||
type: 'text',
|
||||
text,
|
||||
marks: [{ type: 'spoiler' }],
|
||||
};
|
||||
}
|
||||
|
||||
// Text carrying a `comment` mark with an id (the thing that DOES get stripped).
|
||||
function commentedText(text: string, commentId: string) {
|
||||
return {
|
||||
type: 'text',
|
||||
text,
|
||||
marks: [{ type: 'comment', attrs: { commentId, resolved: false } }],
|
||||
};
|
||||
}
|
||||
|
||||
async function sanitize(content: any) {
|
||||
const service = buildService();
|
||||
return service.updatePublicAttachments({
|
||||
id: PAGE,
|
||||
workspaceId: WS,
|
||||
content,
|
||||
} as any);
|
||||
}
|
||||
|
||||
function countMarks(doc: any, type: string): number {
|
||||
let count = 0;
|
||||
const walk = (node: any) => {
|
||||
if (!node || typeof node !== 'object') return;
|
||||
if (Array.isArray(node.marks)) {
|
||||
for (const mark of node.marks) {
|
||||
if (mark?.type === type) count++;
|
||||
}
|
||||
}
|
||||
if (Array.isArray(node.content)) node.content.forEach(walk);
|
||||
};
|
||||
walk(doc);
|
||||
return count;
|
||||
}
|
||||
|
||||
describe('ShareService keeps spoiler marks on public shares (real code)', () => {
|
||||
it('does NOT strip a spoiler mark', async () => {
|
||||
const content = {
|
||||
type: 'doc',
|
||||
content: [
|
||||
{
|
||||
type: 'paragraph',
|
||||
content: [{ type: 'text', text: 'visible ' }, spoilerText('hidden')],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
expect(countMarks(content, 'spoiler')).toBe(1);
|
||||
|
||||
const out = await sanitize(content);
|
||||
|
||||
// The spoiler mark survives the share-strip.
|
||||
expect(countMarks(out, 'spoiler')).toBe(1);
|
||||
expect(JSON.stringify(out)).toContain('hidden');
|
||||
});
|
||||
|
||||
it('strips comment marks but keeps spoiler marks in the same doc', async () => {
|
||||
const content = {
|
||||
type: 'doc',
|
||||
content: [
|
||||
{
|
||||
type: 'paragraph',
|
||||
content: [
|
||||
commentedText('reviewed', 'cmt-1'),
|
||||
{ type: 'text', text: ' and ' },
|
||||
spoilerText('secret'),
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
expect(countMarks(content, 'comment')).toBe(1);
|
||||
expect(countMarks(content, 'spoiler')).toBe(1);
|
||||
|
||||
const out = await sanitize(content);
|
||||
|
||||
// comment is removed, spoiler is preserved.
|
||||
expect(countMarks(out, 'comment')).toBe(0);
|
||||
expect(countMarks(out, 'spoiler')).toBe(1);
|
||||
const serialized = JSON.stringify(out);
|
||||
expect(serialized).not.toContain('cmt-1');
|
||||
expect(serialized).toContain('secret');
|
||||
});
|
||||
});
|
||||
167
apps/server/src/database/repos/page/page.repo.embeddable.spec.ts
Normal file
167
apps/server/src/database/repos/page/page.repo.embeddable.spec.ts
Normal file
@@ -0,0 +1,167 @@
|
||||
import { PageRepo } from './page.repo';
|
||||
import {
|
||||
DummyDriver,
|
||||
Kysely,
|
||||
PostgresAdapter,
|
||||
PostgresIntrospector,
|
||||
PostgresQueryCompiler,
|
||||
} from 'kysely';
|
||||
|
||||
/**
|
||||
* F6 regression guard for the embeddable-page predicate.
|
||||
*
|
||||
* The predicate is shared by `countEmbeddablePages` (the "Indexed N of M" coverage
|
||||
* denominator) and `getEmbeddablePageIds` (the exact set a full reindex iterates).
|
||||
* It MUST select pages whose `text_content` was never backfilled (null/empty) but
|
||||
* whose ProseMirror `content` JSON still carries body text — `reindexPage` builds
|
||||
* its chunks straight from `content`, so without a content clause such a page is
|
||||
* silently SKIPPED by a mass reindex even though it is fully embeddable.
|
||||
*
|
||||
* The content clause keys on the structural text-node marker `"type":"text"`, NOT
|
||||
* a bare `"text":` key. The bare key also appears as the `attrs.text` of atom
|
||||
* nodes that carry NO extractable text — notably math (`mathBlock`/`mathInline`),
|
||||
* whose LaTeX lives in `attrs.text` and has no `generateText` serializer. A
|
||||
* math-ONLY page therefore yields empty `text_content` and zero embeddings; if the
|
||||
* predicate matched its `attrs.text` it would land in the denominator but
|
||||
* `reindexPage` would no-op on it, pinning "Indexed N of M" below 100% forever —
|
||||
* the exact bug this feature fixes. The `"type":"text"` marker matches only real
|
||||
* text nodes (what `jsonToText` extracts), keeping the predicate consistent with
|
||||
* what gets indexed.
|
||||
*
|
||||
* There is no real Postgres here: a recording Kysely (DummyDriver wired to the
|
||||
* Postgres query compiler) compiles the queries to SQL so we can assert the WHERE
|
||||
* predicate ORs in the narrowed content clause alongside the existing text_content
|
||||
* and stored-embeddings clauses — and that BOTH callers compile the identical
|
||||
* clause (denominator and reindex set can never diverge).
|
||||
*/
|
||||
function makeRecordingDb() {
|
||||
const sqls: string[] = [];
|
||||
const db = new Kysely<any>({
|
||||
dialect: {
|
||||
createAdapter: () => new PostgresAdapter(),
|
||||
createDriver: () =>
|
||||
new (class extends DummyDriver {
|
||||
async acquireConnection() {
|
||||
return {
|
||||
executeQuery: async (compiled: { sql: string }) => {
|
||||
sqls.push(compiled.sql);
|
||||
return { rows: [] };
|
||||
},
|
||||
// eslint-disable-next-line @typescript-eslint/no-empty-function
|
||||
streamQuery: async function* () {},
|
||||
} as any;
|
||||
}
|
||||
})(),
|
||||
createIntrospector: (d: Kysely<any>) => new PostgresIntrospector(d),
|
||||
createQueryCompiler: () => new PostgresQueryCompiler(),
|
||||
},
|
||||
});
|
||||
return { db, sqls };
|
||||
}
|
||||
|
||||
// The narrowed content clause, as it appears in the compiled SQL. Keying on the
|
||||
// structural `"type":"text"` marker (not a bare `"text":` key) is what excludes
|
||||
// math-only pages whose only `"text"` key is the atom node's `attrs.text`.
|
||||
const NARROWED_CLAUSE = `"type"[[:space:]]*:[[:space:]]*"text"`;
|
||||
const BARE_TEXT_KEY = `"text"[[:space:]]*:`;
|
||||
|
||||
describe('PageRepo embeddable predicate — content-bearing pages (F6)', () => {
|
||||
it('selects content-bearing pages via the narrowed "type":"text" node marker', async () => {
|
||||
const { db, sqls } = makeRecordingDb();
|
||||
const repo = new PageRepo(db as any, {} as any, { emit: jest.fn() } as any);
|
||||
|
||||
await repo.getEmbeddablePageIds('ws-1');
|
||||
|
||||
expect(sqls).toHaveLength(1);
|
||||
const sql = sqls[0];
|
||||
|
||||
// Clause 1 (existing): pages with extractable text_content.
|
||||
expect(sql).toContain('text_content');
|
||||
// Clause 3 (the F6 fix, now narrowed): a page whose content JSON carries a
|
||||
// real text node is selected even when text_content is null/empty, so a full
|
||||
// reindex visits it instead of silently skipping it.
|
||||
expect(sql).toContain('content::text');
|
||||
expect(sql).toContain(NARROWED_CLAUSE);
|
||||
// It must NOT use the old bare `"text":` key, which also matches the
|
||||
// `attrs.text` of math-only atom pages (false-positive denominator inflation).
|
||||
expect(sql).not.toContain(BARE_TEXT_KEY);
|
||||
// Clause 2 (existing): pages that already have stored embeddings stay in the
|
||||
// set so a reindex can clear their stale rows.
|
||||
expect(sql.toLowerCase()).toContain('embeddings');
|
||||
});
|
||||
|
||||
it('countEmbeddablePages compiles the SAME narrowed clause as getEmbeddablePageIds', async () => {
|
||||
// Consistency is the core requirement: the denominator (countEmbeddablePages)
|
||||
// and the reindex set (getEmbeddablePageIds) MUST share the identical
|
||||
// predicate, else the live "done" counter and the steady-state total diverge.
|
||||
const { db, sqls } = makeRecordingDb();
|
||||
const repo = new PageRepo(db as any, {} as any, { emit: jest.fn() } as any);
|
||||
|
||||
await repo.countEmbeddablePages('ws-1');
|
||||
await repo.getEmbeddablePageIds('ws-1');
|
||||
|
||||
expect(sqls).toHaveLength(2);
|
||||
const [countSql, idsSql] = sqls;
|
||||
|
||||
// Both carry the narrowed content clause...
|
||||
expect(countSql).toContain(NARROWED_CLAUSE);
|
||||
expect(idsSql).toContain(NARROWED_CLAUSE);
|
||||
// ...neither carries the bare key...
|
||||
expect(countSql).not.toContain(BARE_TEXT_KEY);
|
||||
expect(idsSql).not.toContain(BARE_TEXT_KEY);
|
||||
// ...and the full OR predicate (text_content + content node + embeddings
|
||||
// EXISTS) is byte-identical between the two queries, so they can't drift.
|
||||
const where = (s: string) => s.slice(s.indexOf('where'));
|
||||
expect(where(countSql)).toEqual(where(idsSql));
|
||||
});
|
||||
|
||||
it('the content regex matches a text-bearing doc but NOT a math-only doc', () => {
|
||||
// Semantic check of the predicate against sample `content::text` payloads.
|
||||
// Note: `jsonb::text` is NOT identical to JSON.stringify — Postgres renders a
|
||||
// space after each colon (`"type": "text"`), which is exactly why the POSIX
|
||||
// clause uses `[[:space:]]*`. The clause `"type"[[:space:]]*:[[:space:]]*"text"`
|
||||
// maps to the JS regex below (`[[:space:]]` -> `\s`, tolerating both forms);
|
||||
// we evaluate it the way Postgres would.
|
||||
const re = /"type"\s*:\s*"text"/;
|
||||
|
||||
// A real paragraph with a text node -> embeddable.
|
||||
const textDoc = JSON.stringify({
|
||||
type: 'doc',
|
||||
content: [
|
||||
{
|
||||
type: 'paragraph',
|
||||
content: [{ type: 'text', text: 'hello world' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
// A doc whose ONLY node is a math atom. Its LaTeX is in `attrs.text`, there is
|
||||
// no text node, and `jsonToText`/`generateText` has no serializer for it -> it
|
||||
// yields empty text_content and zero embeddings, so it must NOT qualify.
|
||||
const mathOnlyDoc = JSON.stringify({
|
||||
type: 'doc',
|
||||
content: [
|
||||
{ type: 'mathBlock', attrs: { text: 'E = mc^2' } },
|
||||
{ type: 'mathInline', attrs: { text: '\\alpha' } },
|
||||
],
|
||||
});
|
||||
// An empty doc has no text node either.
|
||||
const emptyDoc = JSON.stringify({ type: 'doc', content: [] });
|
||||
|
||||
expect(re.test(textDoc)).toBe(true);
|
||||
expect(re.test(mathOnlyDoc)).toBe(false);
|
||||
expect(re.test(emptyDoc)).toBe(false);
|
||||
// Sanity: the OLD bare-key regex WOULD have wrongly matched the math-only doc,
|
||||
// which is precisely the false positive the narrowing removes.
|
||||
expect(/"text"\s*:/.test(mathOnlyDoc)).toBe(true);
|
||||
|
||||
// A user literally TYPING `"type":"text"` in prose can't false-positive on an
|
||||
// otherwise text-less page: in `content::text` the typed value's quotes are
|
||||
// escaped (`\"type\":\"text\"`), so the literal-quote regex does not match the
|
||||
// escaped form. (And such a page is a genuine text node anyway.)
|
||||
const escapedLiteral = JSON.stringify({
|
||||
type: 'doc',
|
||||
content: [{ type: 'someAtom', attrs: { note: '"type":"text"' } }],
|
||||
});
|
||||
expect(re.test(escapedLiteral)).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -12,6 +12,7 @@ import { executeWithCursorPagination } from '@docmost/db/pagination/cursor-pagin
|
||||
import { validate as isValidUUID } from 'uuid';
|
||||
import { ExpressionBuilder, sql } from 'kysely';
|
||||
import { DB } from '@docmost/db/types/db';
|
||||
import { DbInterface } from '@docmost/db/types/db.interface';
|
||||
import { jsonArrayFrom, jsonObjectFrom } from 'kysely/helpers/postgres';
|
||||
import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo';
|
||||
import { EventEmitter2 } from '@nestjs/event-emitter';
|
||||
@@ -233,9 +234,9 @@ export class PageRepo {
|
||||
* text-less pages (which legitimately store zero embeddings) don't keep the
|
||||
* bar below 100% forever.
|
||||
*
|
||||
* A page qualifies if it has non-empty textContent OR already has stored
|
||||
* embeddings. The second clause covers pages whose text the indexer extracted
|
||||
* from the content JSON when textContent was null, and guarantees this total is
|
||||
* A page qualifies if it has non-empty textContent, OR its content JSON has at
|
||||
* least one text node (`"type":"text"`) when textContent was never backfilled,
|
||||
* OR it already has stored embeddings. The last clause guarantees this total is
|
||||
* always >= countIndexedPages (the indexed count can never exceed it).
|
||||
*/
|
||||
async countEmbeddablePages(workspaceId: string): Promise<number> {
|
||||
@@ -243,37 +244,91 @@ export class PageRepo {
|
||||
.selectFrom('pages as p')
|
||||
.where('p.workspaceId', '=', workspaceId)
|
||||
.where('p.deletedAt', 'is', null)
|
||||
.where((eb) =>
|
||||
eb.or([
|
||||
// Has extractable body text. The regex matches any non-whitespace
|
||||
// character, mirroring the indexer's `text.trim().length === 0` check
|
||||
// (raw SQL -> use the snake_case column name).
|
||||
sql<boolean>`p.text_content ~ '[^[:space:]]'`,
|
||||
// OR already has at least one (non-deleted) embedding row.
|
||||
eb.exists(
|
||||
eb
|
||||
.selectFrom('pageEmbeddings as pe')
|
||||
.select(sql`1`.as('one'))
|
||||
.whereRef('pe.pageId', '=', 'p.id')
|
||||
.where('pe.deletedAt', 'is', null),
|
||||
),
|
||||
]),
|
||||
)
|
||||
.where((eb) => this.embeddablePredicate(eb))
|
||||
.select((eb) => eb.fn.countAll().as('count'))
|
||||
.executeTakeFirst();
|
||||
return Number(row?.count ?? 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* IDs of all non-deleted pages in a workspace. Used by the RAG bulk reindex to
|
||||
* (re)build embeddings for every existing page.
|
||||
* The "embeddable content" qualifying predicate, shared verbatim by
|
||||
* countEmbeddablePages (the steady-state denominator) and getEmbeddablePageIds
|
||||
* (the set the bulk reindex iterates). Both MUST use the exact same condition
|
||||
* or the live total and steady-state total diverge — extracting it here is what
|
||||
* guarantees that, replacing the previous hand-duplicated copy. Callers supply
|
||||
* the trivial workspaceId/deletedAt filters inline; this returns only the
|
||||
* non-trivial OR clause, evaluated against the `p` alias of `pages`.
|
||||
*
|
||||
* A page qualifies if it has non-empty textContent, OR its ProseMirror
|
||||
* `content` JSON has at least one text node (`"type":"text"`) even though
|
||||
* textContent was never backfilled, OR it already has a stored (non-deleted)
|
||||
* embedding row.
|
||||
*/
|
||||
async getIdsByWorkspace(workspaceId: string): Promise<string[]> {
|
||||
private embeddablePredicate(
|
||||
eb: ExpressionBuilder<DbInterface & { p: DbInterface['pages'] }, 'p'>,
|
||||
) {
|
||||
return eb.or([
|
||||
// Has extractable body text. The regex matches any non-whitespace
|
||||
// character, mirroring the indexer's `text.trim().length === 0` check
|
||||
// (raw SQL -> use the snake_case column name).
|
||||
sql<boolean>`p.text_content ~ '[^[:space:]]'`,
|
||||
// OR the ProseMirror `content` JSON has at least one text node (`"type":
|
||||
// "text"`) the indexer can extract, even when `text_content` is null/empty
|
||||
// (never backfilled): `reindexPage` runs `jsonToText` (generateText) over
|
||||
// `content`, which only emits the text of ProseMirror text nodes, so such a
|
||||
// page IS embeddable and a full reindex MUST visit it (otherwise it is
|
||||
// silently skipped). A text node always serialises as
|
||||
// `{"type":"text","text":"..."}`, so we key on the structural `"type":
|
||||
// "text"` marker — NOT a bare `"text":` key, which also appears as the
|
||||
// `attrs.text` of atom nodes that carry NO extractable text (e.g. math
|
||||
// `mathBlock`/`mathInline`, whose LaTeX lives in `attrs.text` and has no
|
||||
// text serializer). A math-only page thus produces empty `text_content` and
|
||||
// zero embeddings; matching its `attrs.text` here would wrongly inflate the
|
||||
// denominator and keep "Indexed N of M" below 100% forever. An empty doc
|
||||
// (no text nodes) has no `"type":"text"` and is correctly excluded. A user
|
||||
// who literally types `"type":"text"` in their prose can't false-positive:
|
||||
// in `content::text` that text value's quotes are escaped (`\"type\"...`),
|
||||
// so the literal-quote regex won't match the escaped form (and such a page
|
||||
// is a real text node anyway).
|
||||
sql<boolean>`p.content::text ~ '"type"[[:space:]]*:[[:space:]]*"text"'`,
|
||||
// OR already has at least one (non-deleted) embedding row.
|
||||
eb.exists(
|
||||
eb
|
||||
.selectFrom('pageEmbeddings as pe')
|
||||
.select(sql`1`.as('one'))
|
||||
.whereRef('pe.pageId', '=', 'p.id')
|
||||
.where('pe.deletedAt', 'is', null),
|
||||
),
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* IDs of the EMBEDDABLE page set for a workspace — the exact same set that
|
||||
* `countEmbeddablePages` counts (a page qualifies if it has non-empty
|
||||
* textContent, OR content JSON with at least one text node (`"type":"text"`)
|
||||
* and an empty/null textContent, OR already has a stored embedding row). The
|
||||
* bulk reindex
|
||||
* iterates THIS set so the live "done" counter reaches exactly
|
||||
* `countEmbeddablePages` (the steady-state denominator), instead of iterating
|
||||
* every non-deleted page (which would push the denominator above the
|
||||
* steady-state value mid-run).
|
||||
*
|
||||
* IMPORTANT: the qualifying WHERE is shared with `countEmbeddablePages` via the
|
||||
* private `embeddablePredicate` helper, so the two can no longer drift — if the
|
||||
* embeddable definition changes, change it once there and both stay in lockstep
|
||||
* (else the live total and steady-state total diverge again). Dropping
|
||||
* text-less pages is correct: `reindexPage` no-ops on
|
||||
* a page with no extractable content anyway, and a page that lost its text but
|
||||
* still has stale embeddings IS in this set (the EXISTS clause), so it is still
|
||||
* visited and its stale rows are cleared.
|
||||
*/
|
||||
async getEmbeddablePageIds(workspaceId: string): Promise<string[]> {
|
||||
const rows = await this.db
|
||||
.selectFrom('pages')
|
||||
.select('id')
|
||||
.where('workspaceId', '=', workspaceId)
|
||||
.where('deletedAt', 'is', null)
|
||||
.selectFrom('pages as p')
|
||||
.select('p.id')
|
||||
.where('p.workspaceId', '=', workspaceId)
|
||||
.where('p.deletedAt', 'is', null)
|
||||
.where((eb) => this.embeddablePredicate(eb))
|
||||
.execute();
|
||||
return rows.map((r) => r.id);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,12 @@
|
||||
import { parsePositiveInt } from './ai-settings.service';
|
||||
import { AiSettingsService, parsePositiveInt } from './ai-settings.service';
|
||||
import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo';
|
||||
import { AiAgentRoleRepo } from '@docmost/db/repos/ai-agent-roles/ai-agent-roles.repo';
|
||||
import { AiProviderCredentialsRepo } from '@docmost/db/repos/ai-chat/ai-provider-credentials.repo';
|
||||
import { PageEmbeddingRepo } from '@docmost/db/repos/ai-chat/page-embedding.repo';
|
||||
import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
||||
import { SecretBoxService } from '../crypto/secret-box';
|
||||
import { EmbeddingReindexProgressService } from './embedding-reindex-progress.service';
|
||||
import type { Queue } from 'bullmq';
|
||||
|
||||
/**
|
||||
* Round-trip coercion for numeric `::text` provider settings (e.g.
|
||||
@@ -41,3 +49,196 @@ describe('parsePositiveInt', () => {
|
||||
expect(parsePositiveInt(42)).toBe(42);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* getMasked must surface the LIVE reindex run progress while a reindex is active
|
||||
* (so the "Indexed X of Y" counter can climb 0 -> total), and fall back to the
|
||||
* steady-state DB coverage count (countIndexedPages / countEmbeddablePages) when
|
||||
* no reindex is running. This is the server side of the fix for the counter that
|
||||
* otherwise stays stuck at "478 of 478" the whole reindex.
|
||||
*/
|
||||
describe('AiSettingsService.getMasked reindex progress', () => {
|
||||
const WORKSPACE_ID = 'ws-1';
|
||||
|
||||
function makeService() {
|
||||
// No driver configured -> the credentials lookup is skipped, keeping the
|
||||
// setup minimal; we only care about the indexed/total numbers here.
|
||||
const workspaceRepo = {
|
||||
findById: jest.fn().mockResolvedValue({ settings: {} }),
|
||||
};
|
||||
const aiAgentRoleRepo = {};
|
||||
const aiProviderCredentialsRepo = { find: jest.fn() };
|
||||
const pageEmbeddingRepo = {
|
||||
countIndexedPages: jest.fn().mockResolvedValue(478),
|
||||
};
|
||||
const pageRepo = {
|
||||
countEmbeddablePages: jest.fn().mockResolvedValue(478),
|
||||
};
|
||||
const secretBox = {};
|
||||
const reindexProgress = {
|
||||
get: jest.fn().mockResolvedValue(null),
|
||||
};
|
||||
const aiQueue = {};
|
||||
|
||||
const service = new AiSettingsService(
|
||||
workspaceRepo as unknown as WorkspaceRepo,
|
||||
aiAgentRoleRepo as unknown as AiAgentRoleRepo,
|
||||
aiProviderCredentialsRepo as unknown as AiProviderCredentialsRepo,
|
||||
pageEmbeddingRepo as unknown as PageEmbeddingRepo,
|
||||
pageRepo as unknown as PageRepo,
|
||||
secretBox as unknown as SecretBoxService,
|
||||
reindexProgress as unknown as EmbeddingReindexProgressService,
|
||||
aiQueue as unknown as Queue,
|
||||
);
|
||||
return { service, reindexProgress, pageEmbeddingRepo };
|
||||
}
|
||||
|
||||
it('reports the live run numbers when a reindex progress record is active', async () => {
|
||||
const { service, reindexProgress } = makeService();
|
||||
// Use a progress.total (500) DISTINCT from the DB count (478) so the test
|
||||
// actually pins the progress.total branch rather than coincidentally
|
||||
// matching the DB fallback. With fix #1 the two sources agree in practice,
|
||||
// but getMasked must still return progress.total when a record is active.
|
||||
reindexProgress.get.mockResolvedValue({
|
||||
total: 500,
|
||||
done: 120,
|
||||
startedAt: Date.now(),
|
||||
});
|
||||
|
||||
const masked = await service.getMasked(WORKSPACE_ID);
|
||||
|
||||
expect(masked.indexedPages).toBe(120); // progress.done, not DB 478
|
||||
expect(masked.totalPages).toBe(500); // progress.total, not DB 478
|
||||
expect(masked.reindexing).toBe(true);
|
||||
});
|
||||
|
||||
it('falls back to countIndexedPages when no reindex is active', async () => {
|
||||
const { service, reindexProgress } = makeService();
|
||||
reindexProgress.get.mockResolvedValue(null);
|
||||
|
||||
const masked = await service.getMasked(WORKSPACE_ID);
|
||||
|
||||
expect(masked.indexedPages).toBe(478);
|
||||
expect(masked.totalPages).toBe(478);
|
||||
expect(masked.reindexing).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* reindex() must seed a live progress record (done=0) BEFORE enqueueing so the
|
||||
* first status poll shows 0 — but ONLY when no run is already active, since
|
||||
* aiQueue.add() de-duplicates a running reindex and a re-seed would reset the
|
||||
* visible counter to 0 while the live worker keeps incrementing from its real
|
||||
* position.
|
||||
*/
|
||||
describe('AiSettingsService.reindex progress seed', () => {
|
||||
const WORKSPACE_ID = 'ws-1';
|
||||
|
||||
function makeService() {
|
||||
const order: string[] = [];
|
||||
const aiQueue = {
|
||||
remove: jest.fn().mockResolvedValue(undefined),
|
||||
add: jest.fn().mockImplementation(async () => {
|
||||
order.push('add');
|
||||
}),
|
||||
};
|
||||
const pageRepo = {
|
||||
countEmbeddablePages: jest.fn().mockResolvedValue(478),
|
||||
};
|
||||
const reindexProgress = {
|
||||
// Default: no active run -> seed should happen.
|
||||
get: jest.fn().mockResolvedValue(null),
|
||||
start: jest.fn().mockImplementation(async () => {
|
||||
order.push('start');
|
||||
}),
|
||||
clear: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
|
||||
const service = new AiSettingsService(
|
||||
{} as unknown as WorkspaceRepo,
|
||||
{} as unknown as AiAgentRoleRepo,
|
||||
{} as unknown as AiProviderCredentialsRepo,
|
||||
{} as unknown as PageEmbeddingRepo,
|
||||
pageRepo as unknown as PageRepo,
|
||||
{} as unknown as SecretBoxService,
|
||||
reindexProgress as unknown as EmbeddingReindexProgressService,
|
||||
aiQueue as unknown as Queue,
|
||||
);
|
||||
return { service, aiQueue, pageRepo, reindexProgress, order };
|
||||
}
|
||||
|
||||
it('seeds progress (workspace, count) BEFORE enqueue when no run is active', async () => {
|
||||
const { service, aiQueue, reindexProgress, order } = makeService();
|
||||
|
||||
await service.reindex(WORKSPACE_ID);
|
||||
|
||||
// The pre-seed carries the real page count AND a SHORT ttl (3rd arg) so a
|
||||
// de-duplicated enqueue against a just-finishing job can't leave a phantom
|
||||
// "reindexing: 0 of N" stuck for the full record TTL (F10).
|
||||
expect(reindexProgress.start).toHaveBeenCalledWith(
|
||||
WORKSPACE_ID,
|
||||
478,
|
||||
expect.any(Number),
|
||||
);
|
||||
const ttl = reindexProgress.start.mock.calls[0][2];
|
||||
// Short pre-seed TTL, distinct from the full 1h (3600s) record TTL, but
|
||||
// pinned to the client poll cap (120s) so a still-pending run can't expire
|
||||
// into a false "done" while the client is still polling (F11).
|
||||
expect(ttl).toBe(120);
|
||||
expect(aiQueue.add).toHaveBeenCalledTimes(1);
|
||||
// Seed must precede the enqueue so the first poll already reports done=0.
|
||||
expect(order).toEqual(['start', 'add']);
|
||||
});
|
||||
|
||||
it('does NOT re-seed when a run is already active (mid-run re-trigger)', async () => {
|
||||
const { service, aiQueue, reindexProgress } = makeService();
|
||||
// An active record exists -> a second click must not reset the counter.
|
||||
reindexProgress.get.mockResolvedValue({
|
||||
total: 478,
|
||||
done: 120,
|
||||
startedAt: Date.now(),
|
||||
});
|
||||
|
||||
await service.reindex(WORKSPACE_ID);
|
||||
|
||||
expect(reindexProgress.start).not.toHaveBeenCalled();
|
||||
// The enqueue still runs (and de-duplicates against the active job).
|
||||
expect(aiQueue.add).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('clears the seed it just wrote and re-throws when enqueue fails', async () => {
|
||||
const { service, aiQueue, reindexProgress } = makeService();
|
||||
// This call seeds (get() is null) but the enqueue then blows up
|
||||
// (Redis hiccup/shutdown) -> the worker never runs and never clear()s, so
|
||||
// reindex() must roll back its own seed to avoid a 1h stuck "reindexing".
|
||||
const boom = new Error('redis down');
|
||||
aiQueue.add.mockRejectedValue(boom);
|
||||
|
||||
await expect(service.reindex(WORKSPACE_ID)).rejects.toBe(boom);
|
||||
|
||||
expect(reindexProgress.start).toHaveBeenCalledWith(
|
||||
WORKSPACE_ID,
|
||||
478,
|
||||
expect.any(Number),
|
||||
);
|
||||
expect(reindexProgress.clear).toHaveBeenCalledWith(WORKSPACE_ID);
|
||||
});
|
||||
|
||||
it('does NOT clear a concurrent active run when enqueue fails (no seed)', async () => {
|
||||
const { service, aiQueue, reindexProgress } = makeService();
|
||||
// A run is already active, so THIS call does not seed; if the enqueue then
|
||||
// fails it must NOT wipe the live worker's record.
|
||||
reindexProgress.get.mockResolvedValue({
|
||||
total: 478,
|
||||
done: 120,
|
||||
startedAt: Date.now(),
|
||||
});
|
||||
const boom = new Error('redis down');
|
||||
aiQueue.add.mockRejectedValue(boom);
|
||||
|
||||
await expect(service.reindex(WORKSPACE_ID)).rejects.toBe(boom);
|
||||
|
||||
expect(reindexProgress.start).not.toHaveBeenCalled();
|
||||
expect(reindexProgress.clear).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,6 +8,7 @@ import { AiProviderCredentialsRepo } from '@docmost/db/repos/ai-chat/ai-provider
|
||||
import { PageEmbeddingRepo } from '@docmost/db/repos/ai-chat/page-embedding.repo';
|
||||
import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
||||
import { SecretBoxService } from '../crypto/secret-box';
|
||||
import { EmbeddingReindexProgressService } from './embedding-reindex-progress.service';
|
||||
import {
|
||||
AiDriver,
|
||||
AiProviderSettings,
|
||||
@@ -30,6 +31,30 @@ export function parsePositiveInt(raw: unknown): number | undefined {
|
||||
return Number.isFinite(n) && n > 0 ? Math.floor(n) : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* TTL (seconds) for the enqueue-time progress PRE-SEED written by `reindex()`
|
||||
* before the worker starts. Deliberately SHORT relative to the full 1h record
|
||||
* TTL: if `aiQueue.add()` de-duplicates against a job that is just finishing
|
||||
* (the worker's finally already ran `clear()` but removeOnComplete hasn't yet
|
||||
* removed the job), no new worker runs to overwrite/clear this seed — so this
|
||||
* shorter TTL lets the phantom "reindexing: 0 of N" expire instead of sticking
|
||||
* for the full 1h record TTL. A worker that DOES start re-seeds with the full
|
||||
* TTL, so a real run is unaffected.
|
||||
*
|
||||
* It MUST be >= the client poll cap (REINDEX_POLL_CAP_MS = 120000ms in
|
||||
* ai-provider-settings.tsx) though: the AI_QUEUE worker runs at concurrency 1
|
||||
* and shares the queue with page-level embedding jobs, so a queued reindex can
|
||||
* wait well beyond a few dozen seconds before the worker re-seeds with the full
|
||||
* TTL. If the pre-seed expired while the job is still pending, `get()` returns
|
||||
* null and getMasked() falls back to the steady-state COUNT (indexedPages ==
|
||||
* totalPages, reindexing=false) — the client reads that as "done & fully
|
||||
* indexed", clears its deadline and STOPS polling, so the admin never sees the
|
||||
* real climb. Pinning the pre-seed TTL to the client cap means a deduped phantom
|
||||
* is bounded to ~120s — the same window the client already polls — and a genuine
|
||||
* pending run never expires-into-"done" inside that window.
|
||||
*/
|
||||
const PRE_SEED_TTL_SECONDS = 120;
|
||||
|
||||
/**
|
||||
* Shape of the partial update accepted by `update`. Mirrors the validated
|
||||
* controller DTO. `apiKey` / `embeddingApiKey` are write-only: undefined =
|
||||
@@ -74,6 +99,7 @@ export class AiSettingsService {
|
||||
private readonly pageEmbeddingRepo: PageEmbeddingRepo,
|
||||
private readonly pageRepo: PageRepo,
|
||||
private readonly secretBox: SecretBoxService,
|
||||
private readonly reindexProgress: EmbeddingReindexProgressService,
|
||||
@InjectQueue(QueueName.AI_QUEUE) private readonly aiQueue: Queue,
|
||||
) {}
|
||||
|
||||
@@ -100,21 +126,63 @@ export class AiSettingsService {
|
||||
.remove(`ai-search-disabled-${workspaceId}`)
|
||||
.catch(() => undefined);
|
||||
|
||||
// Seed a live progress record BEFORE enqueueing so the very first status
|
||||
// poll already reports done=0 (the reindex POST returns the PRE-job counts,
|
||||
// so without this seed the first poll would still show "total of total").
|
||||
// `totalPages` uses countEmbeddablePages — the SAME set the worker iterates
|
||||
// and the SAME denominator the status endpoint reports, so the live and
|
||||
// steady-state totals match.
|
||||
//
|
||||
// ONLY seed when no run is active: aiQueue.add() de-duplicates an already-
|
||||
// running reindex, so a mid-run re-trigger (second click / second admin /
|
||||
// second tab) must NOT reset the visible counter to 0 — that would
|
||||
// understate the live worker's real position for the rest of the run. The
|
||||
// worker's own start() at run begin is the single authoritative reset.
|
||||
let seeded = false;
|
||||
if ((await this.reindexProgress.get(workspaceId)) === null) {
|
||||
const totalPages = await this.pageRepo.countEmbeddablePages(workspaceId);
|
||||
// Short TTL (vs the full 1h record TTL): if add() below de-duplicates
|
||||
// against a just-finishing job whose worker already clear()ed but isn't
|
||||
// removed yet, no worker runs to clear this seed — the shorter TTL expires
|
||||
// the phantom record rather than leaving a stuck "reindexing: 0 of N" for
|
||||
// the full record TTL. It is kept >= the client poll cap (120s) so a
|
||||
// genuine but still-pending run never expires into a false "done" while
|
||||
// the client is still polling (see PRE_SEED_TTL_SECONDS).
|
||||
await this.reindexProgress.start(
|
||||
workspaceId,
|
||||
totalPages,
|
||||
PRE_SEED_TTL_SECONDS,
|
||||
);
|
||||
seeded = true;
|
||||
}
|
||||
|
||||
const jobId = `ai-reindex-${workspaceId}`;
|
||||
// Clear a prior non-active entry so a stale job can't block this reindex.
|
||||
// A locked/active job is left in place (remove() no-ops) and the add() below
|
||||
// de-duplicates against it, keeping the in-progress pass.
|
||||
await this.aiQueue.remove(jobId).catch(() => undefined);
|
||||
|
||||
await this.aiQueue.add(
|
||||
QueueJob.WORKSPACE_CREATE_EMBEDDINGS,
|
||||
{ workspaceId },
|
||||
{
|
||||
jobId,
|
||||
removeOnComplete: true,
|
||||
removeOnFail: true,
|
||||
},
|
||||
);
|
||||
try {
|
||||
await this.aiQueue.add(
|
||||
QueueJob.WORKSPACE_CREATE_EMBEDDINGS,
|
||||
{ workspaceId },
|
||||
{
|
||||
jobId,
|
||||
removeOnComplete: true,
|
||||
removeOnFail: true,
|
||||
},
|
||||
);
|
||||
} catch (err) {
|
||||
// If the enqueue fails (Redis hiccup/shutdown) the worker never runs, so
|
||||
// its finally->clear() never fires. Roll back the seed WE just wrote so
|
||||
// the status endpoint doesn't report a stuck "reindexing: 0 of N" for the
|
||||
// full TTL. Only clear when this call did the seed — never wipe a
|
||||
// concurrent active run's record (get() was non-null, seeded=false).
|
||||
if (seeded) {
|
||||
await this.reindexProgress.clear(workspaceId);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -253,13 +321,33 @@ export class AiSettingsService {
|
||||
hasSttApiKey = !!creds?.sttApiKeyEnc;
|
||||
}
|
||||
|
||||
// totalPages now counts only pages with embeddable content (non-empty text
|
||||
// or already-stored embeddings), so empty/text-less pages don't keep the
|
||||
// "Indexed N of M pages" bar below 100% forever.
|
||||
const [indexedPages, totalPages] = await Promise.all([
|
||||
this.pageEmbeddingRepo.countIndexedPages(workspaceId),
|
||||
this.pageRepo.countEmbeddablePages(workspaceId),
|
||||
]);
|
||||
// While a reindex run is active, report its LIVE progress (done climbs 0 ->
|
||||
// total) so the settings UI can watch it advance. Read progress FIRST and
|
||||
// short-circuit: this endpoint is polled every ~5s for the whole run, so when
|
||||
// a record is active we skip the two coverage COUNTs entirely (their results
|
||||
// would be discarded anyway). Without the live progress the counter never
|
||||
// drops: the per-page reindex hard-replaces rows in its own small
|
||||
// transaction, so countIndexedPages stays ~= total for the whole run. With no
|
||||
// active record we fall back to the steady-state DB coverage count, which
|
||||
// preserves the existing display and the client's "done == total -> stop
|
||||
// polling" condition (the run ends -> record cleared -> DB count == total).
|
||||
//
|
||||
// The fallback `totalPages` counts only pages with embeddable content
|
||||
// (non-empty text, content-borne text, or already-stored embeddings), so
|
||||
// empty/text-less pages don't keep the "Indexed N of M pages" bar below 100%
|
||||
// forever.
|
||||
const progress = await this.reindexProgress.get(workspaceId);
|
||||
let indexedPages: number;
|
||||
let totalPages: number;
|
||||
if (progress) {
|
||||
indexedPages = progress.done;
|
||||
totalPages = progress.total;
|
||||
} else {
|
||||
[indexedPages, totalPages] = await Promise.all([
|
||||
this.pageEmbeddingRepo.countIndexedPages(workspaceId),
|
||||
this.pageRepo.countEmbeddablePages(workspaceId),
|
||||
]);
|
||||
}
|
||||
|
||||
return {
|
||||
driver: provider.driver,
|
||||
@@ -281,6 +369,8 @@ export class AiSettingsService {
|
||||
hasSttApiKey,
|
||||
indexedPages,
|
||||
totalPages,
|
||||
// Optional hint for the client: a reindex run is currently in progress.
|
||||
reindexing: progress != null,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ import { QueueName } from '../queue/constants';
|
||||
import { AiService } from './ai.service';
|
||||
import { AiSettingsService } from './ai-settings.service';
|
||||
import { AiSettingsController } from './ai-settings.controller';
|
||||
import { EmbeddingReindexProgressService } from './embedding-reindex-progress.service';
|
||||
|
||||
/**
|
||||
* LLM driver + provider-settings unit (§6.2/§6.4).
|
||||
@@ -19,7 +20,7 @@ import { AiSettingsController } from './ai-settings.controller';
|
||||
BullModule.registerQueue({ name: QueueName.AI_QUEUE }),
|
||||
],
|
||||
controllers: [AiSettingsController],
|
||||
providers: [AiService, AiSettingsService],
|
||||
exports: [AiService, AiSettingsService],
|
||||
providers: [AiService, AiSettingsService, EmbeddingReindexProgressService],
|
||||
exports: [AiService, AiSettingsService, EmbeddingReindexProgressService],
|
||||
})
|
||||
export class AiModule {}
|
||||
|
||||
@@ -146,4 +146,7 @@ export interface MaskedAiSettings {
|
||||
// RAG indexing coverage for the settings UI.
|
||||
indexedPages: number;
|
||||
totalPages: number;
|
||||
// True while a full workspace reindex is actively running (the counts above
|
||||
// then reflect the live run progress rather than the steady-state DB count).
|
||||
reindexing?: boolean;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,179 @@
|
||||
import { EmbeddingReindexProgressService } from './embedding-reindex-progress.service';
|
||||
import type { RedisService } from '@nestjs-labs/nestjs-ioredis';
|
||||
import type { Redis } from 'ioredis';
|
||||
|
||||
/**
|
||||
* Unit tests for the Redis-backed reindex-progress store.
|
||||
*
|
||||
* The store is a thin, BEST-EFFORT wrapper: writes (start/increment) issue an
|
||||
* hset/hincrby + expire pipeline and must SWALLOW Redis errors (progress is
|
||||
* cosmetic — it must never break a reindex); reads (get) must map a valid hash
|
||||
* to a ReindexProgress and degrade to null on a malformed/missing record or a
|
||||
* Redis failure. We drive it with a hand-rolled fake ioredis (the project mocks
|
||||
* Redis with plain fakes, see public-share limiter specs).
|
||||
*/
|
||||
describe('EmbeddingReindexProgressService', () => {
|
||||
const WORKSPACE_ID = 'ws-1';
|
||||
const KEY = 'ai:reindex:progress:ws-1';
|
||||
|
||||
/**
|
||||
* Build a fake ioredis whose `multi()` returns a chainable recorder and whose
|
||||
* `hgetall`/`del` are configurable jest mocks. `execImpl` lets a test make the
|
||||
* pipeline reject (to assert error-swallowing).
|
||||
*/
|
||||
function makeRedis(opts: { execImpl?: () => Promise<unknown> } = {}) {
|
||||
const exec = jest
|
||||
.fn()
|
||||
.mockImplementation(opts.execImpl ?? (() => Promise.resolve([])));
|
||||
// mockReturnThis() returns the call's `this` (the multi object), so the
|
||||
// chain hset().expire().exec() resolves correctly.
|
||||
const multiObj = {
|
||||
hset: jest.fn().mockReturnThis(),
|
||||
hincrby: jest.fn().mockReturnThis(),
|
||||
expire: jest.fn().mockReturnThis(),
|
||||
exec,
|
||||
};
|
||||
const multi = jest.fn(() => multiObj);
|
||||
const hgetall = jest.fn().mockResolvedValue({});
|
||||
const del = jest.fn().mockResolvedValue(1);
|
||||
const redis = { multi, hgetall, del } as unknown as Redis;
|
||||
return { redis, multiObj, multi, hgetall, del, exec };
|
||||
}
|
||||
|
||||
function makeService(redis: Redis) {
|
||||
const redisService = {
|
||||
getOrThrow: () => redis,
|
||||
} as unknown as RedisService;
|
||||
return new EmbeddingReindexProgressService(redisService);
|
||||
}
|
||||
|
||||
describe('get', () => {
|
||||
it('maps a valid hash to a ReindexProgress object', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
hgetall.mockResolvedValue({ total: '478', done: '120', startedAt: '1000' });
|
||||
const service = makeService(redis);
|
||||
|
||||
await expect(service.get(WORKSPACE_ID)).resolves.toEqual({
|
||||
total: 478,
|
||||
done: 120,
|
||||
startedAt: 1000,
|
||||
});
|
||||
expect(hgetall).toHaveBeenCalledWith(KEY);
|
||||
});
|
||||
|
||||
it('returns null for an empty hash (no record)', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
hgetall.mockResolvedValue({});
|
||||
await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it('returns null when `total` is missing (partial record)', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
hgetall.mockResolvedValue({ done: '5' });
|
||||
await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it('returns null for a non-numeric total', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
hgetall.mockResolvedValue({ total: 'abc', done: '1', startedAt: '1' });
|
||||
await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it('returns null for a non-numeric done', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
hgetall.mockResolvedValue({ total: '10', done: 'xyz', startedAt: '1' });
|
||||
await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it('coerces a non-finite startedAt to 0', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
hgetall.mockResolvedValue({ total: '10', done: '2', startedAt: 'nope' });
|
||||
await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toEqual({
|
||||
total: 10,
|
||||
done: 2,
|
||||
startedAt: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it('degrades to null when hgetall throws (degradation contract)', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
hgetall.mockRejectedValue(new Error('redis down'));
|
||||
await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('start', () => {
|
||||
it('issues hset + expire on the workspace key', async () => {
|
||||
const { redis, multiObj } = makeRedis();
|
||||
await makeService(redis).start(WORKSPACE_ID, 478);
|
||||
|
||||
expect(multiObj.hset).toHaveBeenCalledWith(
|
||||
KEY,
|
||||
expect.objectContaining({ total: '478', done: '0' }),
|
||||
);
|
||||
expect(multiObj.expire).toHaveBeenCalledWith(KEY, expect.any(Number));
|
||||
expect(multiObj.exec).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('defaults the expire TTL to the full 1h record TTL', async () => {
|
||||
const { redis, multiObj } = makeRedis();
|
||||
await makeService(redis).start(WORKSPACE_ID, 478);
|
||||
// Default ttl = full record TTL (60 * 60) so a real run never expires
|
||||
// mid-flight before the worker refreshes it on each increment.
|
||||
expect(multiObj.expire).toHaveBeenCalledWith(KEY, 60 * 60);
|
||||
});
|
||||
|
||||
it('honours an explicit short ttlSeconds for the enqueue-time pre-seed (F10)', async () => {
|
||||
const { redis, multiObj } = makeRedis();
|
||||
// The reindex() pre-seed passes a short ttl so a phantom record left by a
|
||||
// de-duplicated enqueue expires in seconds, not after the full 1h TTL.
|
||||
await makeService(redis).start(WORKSPACE_ID, 478, 45);
|
||||
expect(multiObj.expire).toHaveBeenCalledWith(KEY, 45);
|
||||
});
|
||||
|
||||
it('swallows a thrown Redis error (best-effort)', async () => {
|
||||
const { redis } = makeRedis({
|
||||
execImpl: () => Promise.reject(new Error('redis down')),
|
||||
});
|
||||
await expect(
|
||||
makeService(redis).start(WORKSPACE_ID, 1),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('increment', () => {
|
||||
it('issues hincrby + expire on the workspace key', async () => {
|
||||
const { redis, multiObj } = makeRedis();
|
||||
await makeService(redis).increment(WORKSPACE_ID);
|
||||
|
||||
expect(multiObj.hincrby).toHaveBeenCalledWith(KEY, 'done', 1);
|
||||
expect(multiObj.expire).toHaveBeenCalledWith(KEY, expect.any(Number));
|
||||
expect(multiObj.exec).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('swallows a thrown Redis error (best-effort)', async () => {
|
||||
const { redis } = makeRedis({
|
||||
execImpl: () => Promise.reject(new Error('redis down')),
|
||||
});
|
||||
await expect(
|
||||
makeService(redis).increment(WORKSPACE_ID),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('clear', () => {
|
||||
it('deletes the workspace key', async () => {
|
||||
const { redis, del } = makeRedis();
|
||||
await makeService(redis).clear(WORKSPACE_ID);
|
||||
expect(del).toHaveBeenCalledWith(KEY);
|
||||
});
|
||||
|
||||
it('swallows a thrown Redis error (best-effort)', async () => {
|
||||
const { redis, del } = makeRedis();
|
||||
del.mockRejectedValue(new Error('redis down'));
|
||||
await expect(
|
||||
makeService(redis).clear(WORKSPACE_ID),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,162 @@
|
||||
import { Injectable, Logger } from '@nestjs/common';
|
||||
import { RedisService } from '@nestjs-labs/nestjs-ioredis';
|
||||
import type { Redis } from 'ioredis';
|
||||
|
||||
/**
|
||||
* Live progress of an in-flight workspace embeddings reindex run.
|
||||
* `total` is the number of pages the run will process, `done` how many it has
|
||||
* already processed (success OR handled failure), `startedAt` the epoch-ms the
|
||||
* record was created.
|
||||
*/
|
||||
export interface ReindexProgress {
|
||||
total: number;
|
||||
done: number;
|
||||
startedAt: number;
|
||||
}
|
||||
|
||||
/** Redis key namespace for the per-workspace reindex-progress record. */
|
||||
const KEY_PREFIX = 'ai:reindex:progress:';
|
||||
|
||||
/**
|
||||
* TTL (seconds) on the progress record so a crashed/aborted worker that never
|
||||
* reaches its `clear()` finally can still self-clean instead of leaving a stuck
|
||||
* "reindexing" state. Refreshed on every increment so a long run never expires
|
||||
* mid-flight; on a crash it disappears within TTL of the last processed page.
|
||||
*
|
||||
* INTENTIONALLY tied to WRITE progress (start/increment) only — never refreshed
|
||||
* on get(). Refreshing on read would keep a dead worker's record alive forever
|
||||
* as long as a client keeps polling (a permanently stuck reindexing:true). The
|
||||
* clear() in the worker's finally handles normal completion; a dead worker's
|
||||
* record expires after TTL, and the client's own poll cap stops polling anyway.
|
||||
*/
|
||||
const TTL_SECONDS = 60 * 60; // 1h
|
||||
|
||||
/**
|
||||
* Cluster-wide store for the live progress of a workspace embeddings reindex.
|
||||
*
|
||||
* The reindex runs in a BullMQ worker (AI_QUEUE) that may be a DIFFERENT process
|
||||
* than the API handling the settings-status GET, so the progress must live in
|
||||
* the shared Redis — we reuse the same global ioredis client (RedisService from
|
||||
* @nestjs-labs/nestjs-ioredis) that backs BullMQ and the other anti-abuse
|
||||
* limiters, adding NO new Redis config.
|
||||
*
|
||||
* Everything here is best-effort and COSMETIC: progress only drives the "Indexed
|
||||
* X of Y" counter while a reindex is running. Any Redis failure degrades to the
|
||||
* existing steady-state behaviour (the status falls back to the DB coverage
|
||||
* count), so reads fail to `null` and writes are swallowed — a reindex must
|
||||
* never break because progress reporting did.
|
||||
*
|
||||
* Stored as a Redis HASH so `done` can be bumped with an atomic HINCRBY (the
|
||||
* worker is the only writer of `done`, but HINCRBY also keeps us off a
|
||||
* read-modify-write race and preserves the other fields).
|
||||
*/
|
||||
@Injectable()
|
||||
export class EmbeddingReindexProgressService {
|
||||
private readonly logger = new Logger(EmbeddingReindexProgressService.name);
|
||||
private readonly redis: Redis;
|
||||
|
||||
constructor(redisService: RedisService) {
|
||||
this.redis = redisService.getOrThrow();
|
||||
}
|
||||
|
||||
private key(workspaceId: string): string {
|
||||
return KEY_PREFIX + workspaceId;
|
||||
}
|
||||
|
||||
/**
|
||||
* Begin (or reset) the progress record for a workspace: `total` pages, `done`
|
||||
* back to 0, `startedAt` now. Called twice for a run, BOTH with the real page
|
||||
* count (countEmbeddablePages) so the two totals coincide: once at reindex
|
||||
* enqueue time (so the very first status poll already reports done=0) and again
|
||||
* at the worker start (which re-asserts the same total and resets `done`).
|
||||
* Resets `done` to 0 so a re-trigger never inherits a stale count.
|
||||
*
|
||||
* `ttlSeconds` lets the caller pick the record's lifetime. The enqueue-time
|
||||
* pre-seed passes a SHORT ttl: if `aiQueue.add()` de-duplicates against a job
|
||||
* that is just finishing (its worker hasn't yet removed the job but already
|
||||
* ran its `clear()`), no new worker starts to clear this phantom seed, so a
|
||||
* short ttl lets it expire in seconds instead of sticking for the full TTL.
|
||||
* The worker's own `start()` at the begin of a real run overwrites this entry
|
||||
* and raises the ttl back to the default full TTL.
|
||||
*/
|
||||
async start(
|
||||
workspaceId: string,
|
||||
total: number,
|
||||
ttlSeconds: number = TTL_SECONDS,
|
||||
): Promise<void> {
|
||||
const key = this.key(workspaceId);
|
||||
try {
|
||||
await this.redis
|
||||
.multi()
|
||||
.hset(key, {
|
||||
total: String(total),
|
||||
done: '0',
|
||||
startedAt: String(Date.now()),
|
||||
})
|
||||
.expire(key, ttlSeconds)
|
||||
.exec();
|
||||
} catch (err) {
|
||||
this.logger.warn(
|
||||
`reindex-progress start failed for workspace ${workspaceId}; ` +
|
||||
`progress reporting disabled for this run: ${(err as Error).message}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Bump the processed-page counter by one and refresh the TTL. Atomic and
|
||||
* best-effort: a missing key (cleared/expired) would be recreated with only
|
||||
* `done`, but `get()` treats a record without a numeric `total` as inactive,
|
||||
* so that partial state safely reads as "no active reindex".
|
||||
*/
|
||||
async increment(workspaceId: string): Promise<void> {
|
||||
const key = this.key(workspaceId);
|
||||
try {
|
||||
await this.redis.multi().hincrby(key, 'done', 1).expire(key, TTL_SECONDS).exec();
|
||||
} catch (err) {
|
||||
this.logger.warn(
|
||||
`reindex-progress increment failed for workspace ${workspaceId}: ` +
|
||||
`${(err as Error).message}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove the progress record. Called in the worker's `finally` so a completed,
|
||||
* aborted, or unconfigured-early-return run never leaves a stuck record; the
|
||||
* status then falls back to the DB coverage count.
|
||||
*/
|
||||
async clear(workspaceId: string): Promise<void> {
|
||||
try {
|
||||
await this.redis.del(this.key(workspaceId));
|
||||
} catch (err) {
|
||||
this.logger.warn(
|
||||
`reindex-progress clear failed for workspace ${workspaceId} ` +
|
||||
`(self-cleans via TTL): ${(err as Error).message}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the live progress, or `null` when no reindex is active (no record, an
|
||||
* expired record, or a partial record without a numeric `total`). On a Redis
|
||||
* error returns `null` so the status endpoint degrades to its DB count.
|
||||
*/
|
||||
async get(workspaceId: string): Promise<ReindexProgress | null> {
|
||||
try {
|
||||
const data = await this.redis.hgetall(this.key(workspaceId));
|
||||
if (!data || data.total === undefined) return null;
|
||||
const total = Number(data.total);
|
||||
const done = Number(data.done);
|
||||
const startedAt = Number(data.startedAt);
|
||||
if (!Number.isFinite(total) || !Number.isFinite(done)) return null;
|
||||
return { total, done, startedAt: Number.isFinite(startedAt) ? startedAt : 0 };
|
||||
} catch (err) {
|
||||
this.logger.warn(
|
||||
`reindex-progress read failed for workspace ${workspaceId}; ` +
|
||||
`falling back to DB count: ${(err as Error).message}`,
|
||||
);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -10,7 +10,6 @@ import {
|
||||
PAGE_TEMPLATE_THROTTLER,
|
||||
PUBLIC_SHARE_AI_THROTTLER,
|
||||
} from './throttler-names';
|
||||
import Redis from 'ioredis';
|
||||
|
||||
@Module({
|
||||
imports: [
|
||||
@@ -32,16 +31,18 @@ import Redis from 'ioredis';
|
||||
{ name: PUBLIC_SHARE_AI_THROTTLER, ttl: 60_000, limit: 5 },
|
||||
],
|
||||
errorMessage: 'Too many requests',
|
||||
storage: new ThrottlerStorageRedisService(
|
||||
new Redis({
|
||||
host: redisConfig.host,
|
||||
port: redisConfig.port,
|
||||
password: redisConfig.password,
|
||||
db: redisConfig.db,
|
||||
family: redisConfig.family,
|
||||
keyPrefix: 'throttle:',
|
||||
}),
|
||||
),
|
||||
// Pass ioredis options (not a pre-built Redis instance) so
|
||||
// ThrottlerStorageRedisService owns the connection and disconnects it
|
||||
// in its onModuleDestroy. Passing an instance leaves disconnectRequired
|
||||
// false, so the socket would leak on shutdown (e2e jest never exits).
|
||||
storage: new ThrottlerStorageRedisService({
|
||||
host: redisConfig.host,
|
||||
port: redisConfig.port,
|
||||
password: redisConfig.password,
|
||||
db: redisConfig.db,
|
||||
family: redisConfig.family,
|
||||
keyPrefix: 'throttle:',
|
||||
}),
|
||||
};
|
||||
},
|
||||
inject: [EnvironmentService],
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { Logger } from '@nestjs/common';
|
||||
import { IoAdapter } from '@nestjs/platform-socket.io';
|
||||
import { ServerOptions } from 'socket.io';
|
||||
import { createAdapter } from '@socket.io/redis-adapter';
|
||||
@@ -9,8 +10,11 @@ import {
|
||||
} from '../../common/helpers';
|
||||
|
||||
export class WsRedisIoAdapter extends IoAdapter {
|
||||
private readonly logger = new Logger(WsRedisIoAdapter.name);
|
||||
private adapterConstructor: ReturnType<typeof createAdapter>;
|
||||
private redisConfig: RedisConfig;
|
||||
private pubClient: Redis;
|
||||
private subClient: Redis;
|
||||
|
||||
async connectToRedis(): Promise<void> {
|
||||
this.redisConfig = parseRedisUrl(process.env.REDIS_URL);
|
||||
@@ -23,8 +27,13 @@ export class WsRedisIoAdapter extends IoAdapter {
|
||||
const pubClient = new Redis(process.env.REDIS_URL, options);
|
||||
const subClient = new Redis(process.env.REDIS_URL, options);
|
||||
|
||||
pubClient.on('error', (err) => () => {});
|
||||
subClient.on('error', (err) => () => {});
|
||||
pubClient.on('error', (err) => this.logger.error('socket.io redis pub client error', err));
|
||||
subClient.on('error', (err) => this.logger.error('socket.io redis sub client error', err));
|
||||
|
||||
// Hold references so the pub/sub connections can be torn down on shutdown
|
||||
// (see dispose()); otherwise these ioredis sockets leak as active handles.
|
||||
this.pubClient = pubClient;
|
||||
this.subClient = subClient;
|
||||
|
||||
this.adapterConstructor = createAdapter(pubClient, subClient);
|
||||
}
|
||||
@@ -34,4 +43,26 @@ export class WsRedisIoAdapter extends IoAdapter {
|
||||
server.adapter(this.adapterConstructor);
|
||||
return server;
|
||||
}
|
||||
|
||||
/**
|
||||
* Called once by Nest's SocketModule during application shutdown, after every
|
||||
* socket.io server has been closed. The @socket.io/redis-adapter never owns
|
||||
* the lifecycle of the ioredis pub/sub clients it is handed, so we close them
|
||||
* here to avoid leaking their TCP handles on shutdown (see issue #255).
|
||||
*
|
||||
* Uses disconnect(false) to mirror the sibling pub/sub pair in
|
||||
* collaboration/extensions/redis-sync (redis-sync.extension.ts onDestroy):
|
||||
* an immediate close with no graceful QUIT round-trip and no auto-reconnect,
|
||||
* which is what we want for idle adapter clients during teardown.
|
||||
*/
|
||||
async dispose(): Promise<void> {
|
||||
await super.dispose();
|
||||
|
||||
// dispose() is invoked once per shutdown; null the refs so a second call
|
||||
// (or any post-shutdown path) cannot act on already-closed clients.
|
||||
this.pubClient?.disconnect(false);
|
||||
this.subClient?.disconnect(false);
|
||||
this.pubClient = undefined;
|
||||
this.subClient = undefined;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,160 @@
|
||||
import { Kysely } from 'kysely';
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
||||
import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo';
|
||||
import { EventEmitter2 } from '@nestjs/event-emitter';
|
||||
import { getTestDb, destroyTestDb, createWorkspace, createSpace } from './db';
|
||||
|
||||
/**
|
||||
* `PageRepo.getEmbeddablePageIds` MUST stay in lockstep with
|
||||
* `PageRepo.countEmbeddablePages` (page.repo.ts) — the bulk reindex iterates the
|
||||
* ID set while the status endpoint reports the count as the live denominator, so
|
||||
* if the two predicates ever diverge the "done X of Y" counter ends on the wrong
|
||||
* total. Both share the SAME WHERE: a page qualifies iff it is non-deleted AND
|
||||
* (text_content has a non-whitespace char OR — when text_content is empty — its
|
||||
* content JSON has a text node OR it has a non-deleted embedding row).
|
||||
*
|
||||
* This is a DB-level invariant: the predicate lives in raw SQL (`text_content ~
|
||||
* '[^[:space:]]'`, `content::text ~ '"type"[[:space:]]*:[[:space:]]*"text"'`) and an EXISTS subquery, so a unit test with mocked Kysely
|
||||
* cannot observe it. We seed every boundary case against real Postgres and
|
||||
* assert the returned ID set EQUALS the count (and is exactly the expected set).
|
||||
* A future edit that touches one predicate but not the other turns this red.
|
||||
*/
|
||||
describe('PageRepo embeddable-page set: getEmbeddablePageIds <-> countEmbeddablePages [integration]', () => {
|
||||
let db: Kysely<any>;
|
||||
let repo: PageRepo;
|
||||
let workspaceId: string;
|
||||
let spaceId: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
db = getTestDb();
|
||||
// Only the Kysely-backed query methods under test are exercised, so the
|
||||
// SpaceMemberRepo / EventEmitter2 deps are never touched — stub them.
|
||||
repo = new PageRepo(
|
||||
db as any,
|
||||
{} as unknown as SpaceMemberRepo,
|
||||
{} as unknown as EventEmitter2,
|
||||
);
|
||||
workspaceId = (await createWorkspace(db)).id;
|
||||
spaceId = (await createSpace(db, workspaceId)).id;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await destroyTestDb();
|
||||
});
|
||||
|
||||
// Insert a page with explicit text_content / content / deleted_at (createPage
|
||||
// in db.ts sets none), returning its id so the test can assert membership.
|
||||
// `content` is the ProseMirror doc JSON (jsonb): postgres.js serializes a plain
|
||||
// object to JSON for jsonb columns, so we pass it through only when supplied so
|
||||
// the rest of the rows keep the DB default.
|
||||
async function insertPage(args: {
|
||||
textContent: string | null;
|
||||
content?: unknown;
|
||||
deletedAt?: Date | null;
|
||||
}): Promise<string> {
|
||||
const id = randomUUID();
|
||||
await db
|
||||
.insertInto('pages')
|
||||
.values({
|
||||
id,
|
||||
slugId: `slug-${id.slice(0, 8)}`,
|
||||
title: `page-${id.slice(0, 8)}`,
|
||||
spaceId,
|
||||
workspaceId,
|
||||
textContent: args.textContent,
|
||||
...(args.content !== undefined ? { content: args.content as any } : {}),
|
||||
deletedAt: args.deletedAt ?? null,
|
||||
})
|
||||
.execute();
|
||||
return id;
|
||||
}
|
||||
|
||||
// Insert one embedding chunk row for a page (NOT NULL columns + deleted_at).
|
||||
async function insertEmbedding(
|
||||
pageId: string,
|
||||
opts: { deletedAt?: Date | null } = {},
|
||||
): Promise<void> {
|
||||
await db
|
||||
.insertInto('pageEmbeddings')
|
||||
.values({
|
||||
id: randomUUID(),
|
||||
workspaceId,
|
||||
pageId,
|
||||
spaceId,
|
||||
chunkIndex: 0,
|
||||
chunkStart: 0,
|
||||
chunkLength: 1,
|
||||
content: 'x',
|
||||
modelName: 'test-model',
|
||||
modelDimensions: 1,
|
||||
deletedAt: opts.deletedAt ?? null,
|
||||
})
|
||||
.execute();
|
||||
}
|
||||
|
||||
it('returns exactly the embeddable set and its size equals countEmbeddablePages', async () => {
|
||||
// IN the set --------------------------------------------------------------
|
||||
// (a) non-deleted page with real body text.
|
||||
const withText = await insertPage({ textContent: 'hello world' });
|
||||
// (b) non-deleted page with NO text but a live embedding row (EXISTS clause:
|
||||
// a page that lost its text yet still has stale vectors must be visited
|
||||
// so the reindex can clear them).
|
||||
const noTextLiveEmbedding = await insertPage({ textContent: null });
|
||||
await insertEmbedding(noTextLiveEmbedding);
|
||||
// (c) non-deleted page with EMPTY text_content but ProseMirror `content` JSON
|
||||
// carrying a real text node — the content-JSON clause. This pins BOTH the
|
||||
// third OR-clause AND the space-after-colon: jsonb stores the key/value
|
||||
// separator as `"type": "text"` (a space after the colon), which is why
|
||||
// the predicate needs `[[:space:]]*`. `reindexPage` extracts this text, so
|
||||
// the page IS embeddable and the reindex MUST visit it.
|
||||
const noTextContentDoc = await insertPage({
|
||||
textContent: null,
|
||||
content: {
|
||||
type: 'doc',
|
||||
content: [
|
||||
{ type: 'paragraph', content: [{ type: 'text', text: 'hello' }] },
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
// OUT of the set ----------------------------------------------------------
|
||||
// (d) non-deleted, text_content NULL, no embeddings.
|
||||
await insertPage({ textContent: null });
|
||||
// (e) non-deleted, whitespace-only text (regex requires a non-space char).
|
||||
await insertPage({ textContent: ' \n\t ' });
|
||||
// (f) deleted page WITH body text — excluded by the non-deleted predicate.
|
||||
await insertPage({
|
||||
textContent: 'deleted but had text',
|
||||
deletedAt: new Date(),
|
||||
});
|
||||
// (g) non-deleted, no text, with ONLY a DELETED embedding row — the EXISTS
|
||||
// subquery filters pe.deleted_at IS NULL, so this stays out.
|
||||
const onlyDeletedEmbedding = await insertPage({ textContent: null });
|
||||
await insertEmbedding(onlyDeletedEmbedding, { deletedAt: new Date() });
|
||||
// (h) non-deleted, empty text_content, content JSON with ONLY a math atom
|
||||
// node — its LaTeX lives in `attrs.text` (a `"text":` KEY, not a
|
||||
// `"type":"text"` text node) and has no text serializer, so `jsonToText`
|
||||
// yields nothing and the page produces zero embeddings. The predicate
|
||||
// keys on the structural `"type":"text"` marker, so this stays OUT (a
|
||||
// bare `"text":` match would wrongly inflate the denominator).
|
||||
await insertPage({
|
||||
textContent: null,
|
||||
content: {
|
||||
type: 'doc',
|
||||
content: [{ type: 'mathBlock', attrs: { text: 'E=mc^2' } }],
|
||||
},
|
||||
});
|
||||
|
||||
const ids = await repo.getEmbeddablePageIds(workspaceId);
|
||||
const count = await repo.countEmbeddablePages(workspaceId);
|
||||
|
||||
// The two queries agree on the size (the load-bearing lockstep invariant)...
|
||||
expect(ids.length).toBe(count);
|
||||
// ...and the set is exactly the three qualifying pages, nothing else.
|
||||
expect(new Set(ids)).toEqual(
|
||||
new Set([withText, noTextLiveEmbedding, noTextContentDoc]),
|
||||
);
|
||||
expect(count).toBe(3);
|
||||
});
|
||||
});
|
||||
@@ -25,6 +25,7 @@ export * from "./lib/subpages";
|
||||
export * from "./lib/transclusion";
|
||||
export * from "./lib/page-embed";
|
||||
export * from "./lib/highlight";
|
||||
export * from "./lib/spoiler/spoiler";
|
||||
export * from "./lib/indent";
|
||||
export * from "./lib/heading/heading";
|
||||
export * from "./lib/unique-id";
|
||||
|
||||
68
packages/editor-ext/src/lib/image/image-markdown.test.ts
Normal file
68
packages/editor-ext/src/lib/image/image-markdown.test.ts
Normal file
@@ -0,0 +1,68 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { generateJSON } from "@tiptap/html";
|
||||
import { Document } from "@tiptap/extension-document";
|
||||
import { Paragraph } from "@tiptap/extension-paragraph";
|
||||
import { Text } from "@tiptap/extension-text";
|
||||
import { htmlToMarkdown } from "../markdown/utils/turndown.utils";
|
||||
import { markdownToHtml } from "../markdown/utils/marked.utils";
|
||||
import { TiptapImage } from "./image";
|
||||
|
||||
// Minimal schema for parsing markdownToHtml output back to JSON (mirrors
|
||||
// image.spec.ts), so we can assert the recovered caption EXACTLY.
|
||||
const parseExtensions = [Document, Paragraph, Text, TiptapImage];
|
||||
|
||||
// Lossless markdown round-trip for image captions (issue #221). An image WITH a
|
||||
// caption can't be expressed as ``, so it is emitted as a raw <img>
|
||||
// (carrying data-caption) wrapped in a block <div>, the same trick the <video>
|
||||
// rule uses. marked passes the raw HTML through, so markdownToHtml keeps the
|
||||
// data-caption, and the image extension's parseHTML restores the attribute.
|
||||
describe("image caption markdown round-trip", () => {
|
||||
it("HTML -> Markdown emits a raw <img data-caption> for captioned images", () => {
|
||||
const html = `<p><img src="/files/a.png" alt="cat" data-caption="A grey cat"></p>`;
|
||||
const md = htmlToMarkdown(html);
|
||||
expect(md).toContain("data-caption=\"A grey cat\"");
|
||||
expect(md).toContain('src="/files/a.png"');
|
||||
expect(md).toContain('alt="cat"');
|
||||
// It must NOT degrade to the lossy ![]() form.
|
||||
expect(md).not.toContain("![cat]");
|
||||
});
|
||||
|
||||
it("Markdown -> HTML restores data-caption on the <img>", async () => {
|
||||
const html = `<p><img src="/files/a.png" alt="cat" data-caption="A grey cat"></p>`;
|
||||
const md = htmlToMarkdown(html);
|
||||
const back = await markdownToHtml(md);
|
||||
expect(back).toContain('data-caption="A grey cat"');
|
||||
expect(back).toContain('src="/files/a.png"');
|
||||
});
|
||||
|
||||
it("special characters in the caption survive the round-trip (escaped)", async () => {
|
||||
// The source caption is the decoded string `Tom & "Jerry"` (both an `&` and
|
||||
// a `"`). escapeHtmlAttr must encode `&` -> `&` and `"` -> `"`.
|
||||
const html = `<p><img src="/files/a.png" data-caption='Tom & "Jerry"'></p>`;
|
||||
const md = htmlToMarkdown(html);
|
||||
|
||||
// (a) The intermediate Markdown must carry the EXACT escaped attribute. This
|
||||
// fails if escapeHtmlAttr stopped escaping `"` (attribute break-out:
|
||||
// data-caption="Tom & "Jerry"") or double-encoded `&` (`&amp;`).
|
||||
expect(md).toContain('data-caption="Tom & "Jerry""');
|
||||
|
||||
const back = await markdownToHtml(md);
|
||||
expect(back).toContain("data-caption=");
|
||||
expect(back).toContain("Jerry");
|
||||
expect(back).toContain("Tom");
|
||||
|
||||
// (b) Re-parse the rendered HTML through the image extension's parseHTML and
|
||||
// assert the recovered caption is EXACTLY the original (no corruption, loss,
|
||||
// or double-encoding).
|
||||
const json = generateJSON(back, parseExtensions);
|
||||
expect(json.content?.[0]?.attrs?.caption).toBe('Tom & "Jerry"');
|
||||
});
|
||||
|
||||
it("caption-less images stay a clean  with no raw HTML", () => {
|
||||
const html = `<p><img src="/files/a.png" alt="cat"></p>`;
|
||||
const md = htmlToMarkdown(html);
|
||||
expect(md).toContain("");
|
||||
expect(md).not.toContain("data-caption");
|
||||
expect(md).not.toContain("<img");
|
||||
});
|
||||
});
|
||||
@@ -1,5 +1,16 @@
|
||||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import { applyAlignment } from "./image";
|
||||
import { getSchema } from "@tiptap/core";
|
||||
import { generateHTML, generateJSON } from "@tiptap/html";
|
||||
import { Document } from "@tiptap/extension-document";
|
||||
import { Paragraph } from "@tiptap/extension-paragraph";
|
||||
import { Text } from "@tiptap/extension-text";
|
||||
import { applyAlignment, TiptapImage } from "./image";
|
||||
|
||||
// CONTRACT tests for the image node's `caption` attribute (issue #221). The
|
||||
// caption is a plain-text string stored on the image atom and serialized as
|
||||
// `data-caption` on the <img>. If this mapping drifts, captions saved to HTML
|
||||
// (and thus to native storage / search / markdown) are silently lost.
|
||||
const extensions = [Document, Paragraph, Text, TiptapImage];
|
||||
|
||||
// applyAlignment is a pure DOM mutation: it sets the float / padding /
|
||||
// justify-content / data-image-align on an image node-view container per the
|
||||
@@ -65,3 +76,56 @@ describe("applyAlignment", () => {
|
||||
expect(el.style.justifyContent).toBe("flex-start");
|
||||
});
|
||||
});
|
||||
|
||||
describe("image schema", () => {
|
||||
it("registers the image node and keeps it an atom", () => {
|
||||
const schema = getSchema(extensions);
|
||||
expect(schema.nodes.image).toBeTruthy();
|
||||
expect(schema.nodes.image.spec.atom).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("image caption parse/render round-trip", () => {
|
||||
it("recovers caption from data-caption on parse (HTML -> JSON)", () => {
|
||||
const html = `<img src="/files/a.png" alt="cat" data-caption="A grey cat">`;
|
||||
const json = generateJSON(html, extensions);
|
||||
|
||||
const node = json.content?.[0];
|
||||
expect(node?.type).toBe("image");
|
||||
expect(node?.attrs?.caption).toBe("A grey cat");
|
||||
expect(node?.attrs?.alt).toBe("cat");
|
||||
});
|
||||
|
||||
it("emits data-caption on render when set (JSON -> HTML)", () => {
|
||||
const json = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "image",
|
||||
attrs: { src: "/files/a.png", alt: "cat", caption: "A grey cat" },
|
||||
},
|
||||
],
|
||||
};
|
||||
const html = generateHTML(json, extensions);
|
||||
expect(html).toContain('data-caption="A grey cat"');
|
||||
});
|
||||
|
||||
it("omits data-caption when there is no caption (caption-less images stay clean)", () => {
|
||||
const json = {
|
||||
type: "doc",
|
||||
content: [{ type: "image", attrs: { src: "/files/a.png", alt: "cat" } }],
|
||||
};
|
||||
const html = generateHTML(json, extensions);
|
||||
expect(html).not.toContain("data-caption");
|
||||
});
|
||||
|
||||
it("full HTML -> JSON -> HTML round-trip preserves the caption", () => {
|
||||
const html = `<img src="/files/a.png" alt="cat" data-caption="Caption with & "quotes"">`;
|
||||
const json = generateJSON(html, extensions);
|
||||
expect(json.content?.[0]?.attrs?.caption).toBe('Caption with & "quotes"');
|
||||
|
||||
const out = generateHTML(json, extensions);
|
||||
const back = generateJSON(out, extensions);
|
||||
expect(back.content?.[0]?.attrs?.caption).toBe('Caption with & "quotes"');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -32,6 +32,7 @@ export interface ImageOptions extends DefaultImageOptions {
|
||||
export interface ImageAttributes {
|
||||
src?: string;
|
||||
alt?: string;
|
||||
caption?: string;
|
||||
align?: string;
|
||||
attachmentId?: string;
|
||||
size?: number;
|
||||
@@ -125,6 +126,13 @@ export const TiptapImage = Image.extend<ImageOptions>({
|
||||
alt: attributes.alt,
|
||||
}),
|
||||
},
|
||||
caption: {
|
||||
default: undefined,
|
||||
parseHTML: (element) => element.getAttribute("data-caption") || undefined,
|
||||
// Emit data-caption only when set, so caption-less images stay clean.
|
||||
renderHTML: (attributes: ImageAttributes) =>
|
||||
attributes.caption ? { "data-caption": attributes.caption } : {},
|
||||
},
|
||||
attachmentId: {
|
||||
default: undefined,
|
||||
parseHTML: (element) => element.getAttribute("data-attachment-id"),
|
||||
@@ -304,6 +312,10 @@ export const TiptapImage = Image.extend<ImageOptions>({
|
||||
el.alt = updatedNode.attrs.alt || "";
|
||||
}
|
||||
|
||||
if (updatedNode.attrs.caption !== currentNode.attrs.caption) {
|
||||
applyCaption(updatedNode.attrs.caption);
|
||||
}
|
||||
|
||||
const w = updatedNode.attrs.width;
|
||||
const h = updatedNode.attrs.height;
|
||||
if (w != null) {
|
||||
@@ -335,6 +347,28 @@ export const TiptapImage = Image.extend<ImageOptions>({
|
||||
|
||||
const dom = nodeView.dom as HTMLElement;
|
||||
|
||||
// Re-parent the resizable wrapper into a <figure> so the caption sits BELOW
|
||||
// the image, OUTSIDE nodeView.wrapper. onCommit measures the img's
|
||||
// offsetHeight for the persisted height/aspectRatio, and the left/right
|
||||
// resize handles span the wrapper — both must cover the image only. The
|
||||
// <figure> stays the single flex child of the container, so applyAlignment
|
||||
// and the float modes keep working. This path also drives read-only/share.
|
||||
const figure = document.createElement("figure");
|
||||
figure.style.margin = "0";
|
||||
figure.style.display = "inline-block"; // shrink-to-fit to image width
|
||||
figure.appendChild(nodeView.wrapper);
|
||||
dom.appendChild(figure);
|
||||
|
||||
const figcaption = document.createElement("figcaption");
|
||||
figcaption.className = "image-caption";
|
||||
const applyCaption = (text?: string) => {
|
||||
const value = (text || "").trim();
|
||||
figcaption.textContent = value;
|
||||
figcaption.style.display = value ? "block" : "none";
|
||||
};
|
||||
applyCaption(node.attrs.caption);
|
||||
figure.appendChild(figcaption);
|
||||
|
||||
// Apply initial alignment
|
||||
applyAlignment(dom, node.attrs.align || "center");
|
||||
|
||||
|
||||
@@ -0,0 +1,128 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { getSchema } from "@tiptap/core";
|
||||
import { generateHTML, generateJSON } from "@tiptap/html";
|
||||
import { Document } from "@tiptap/extension-document";
|
||||
import { Paragraph } from "@tiptap/extension-paragraph";
|
||||
import { Text } from "@tiptap/extension-text";
|
||||
import { Bold } from "@tiptap/extension-bold";
|
||||
import { htmlToMarkdown } from "./turndown.utils";
|
||||
import { markdownToHtml } from "./marked.utils";
|
||||
import { Spoiler } from "../../spoiler/spoiler";
|
||||
|
||||
// The spoiler mark has no native Markdown syntax, so it is preserved losslessly
|
||||
// as raw inline HTML (`<span data-spoiler="true">…</span>`), the same approach
|
||||
// htmlEmbed uses. This test drives the full editor round-trip:
|
||||
// JSON -> HTML -> Markdown -> HTML -> JSON
|
||||
// and asserts the `spoiler` mark survives end to end. We use the same
|
||||
// getSchema + @tiptap/html generateHTML/generateJSON utilities the other
|
||||
// editor-ext schema tests use.
|
||||
|
||||
const extensions = [Document, Paragraph, Text, Bold, Spoiler];
|
||||
|
||||
function html(md: string): string {
|
||||
const out = markdownToHtml(md);
|
||||
if (typeof out !== "string") throw new Error("expected sync string output");
|
||||
return out;
|
||||
}
|
||||
|
||||
// Count text nodes carrying a `spoiler` mark anywhere in a ProseMirror JSON doc.
|
||||
function countSpoilerMarks(doc: any): number {
|
||||
let count = 0;
|
||||
const walk = (node: any) => {
|
||||
if (!node || typeof node !== "object") return;
|
||||
if (Array.isArray(node.marks)) {
|
||||
for (const mark of node.marks) {
|
||||
if (mark?.type === "spoiler") count++;
|
||||
}
|
||||
}
|
||||
if (Array.isArray(node.content)) node.content.forEach(walk);
|
||||
};
|
||||
walk(doc);
|
||||
return count;
|
||||
}
|
||||
|
||||
describe("Spoiler mark schema", () => {
|
||||
it("registers the spoiler mark in the schema", () => {
|
||||
const schema = getSchema(extensions);
|
||||
expect(schema.marks.spoiler).toBeTruthy();
|
||||
});
|
||||
|
||||
it("recovers the spoiler mark from span[data-spoiler] (HTML -> JSON)", () => {
|
||||
const json = generateJSON(
|
||||
'<p>before <span data-spoiler="true">hidden</span> after</p>',
|
||||
extensions,
|
||||
);
|
||||
expect(countSpoilerMarks(json)).toBe(1);
|
||||
});
|
||||
|
||||
it("emits data-spoiler + class on render (JSON -> HTML)", () => {
|
||||
const doc = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "hidden",
|
||||
marks: [{ type: "spoiler" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
const out = generateHTML(doc, extensions);
|
||||
expect(out).toContain('data-spoiler="true"');
|
||||
expect(out).toContain('class="spoiler"');
|
||||
});
|
||||
});
|
||||
|
||||
describe("Spoiler Markdown round-trip is lossless", () => {
|
||||
const docWith = (textNode: any) => ({
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
content: [{ type: "text", text: "before " }, textNode, { type: "text", text: " after" }],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
it("preserves the spoiler mark through JSON -> MD -> HTML -> JSON", () => {
|
||||
const startDoc = docWith({
|
||||
type: "text",
|
||||
text: "hidden",
|
||||
marks: [{ type: "spoiler" }],
|
||||
});
|
||||
|
||||
// JSON -> HTML
|
||||
const html1 = generateHTML(startDoc, extensions);
|
||||
expect(html1).toContain('data-spoiler="true"');
|
||||
|
||||
// HTML -> Markdown (raw inline HTML, lossless)
|
||||
const md = htmlToMarkdown(html1);
|
||||
expect(md).toContain('<span data-spoiler="true">hidden</span>');
|
||||
|
||||
// MD -> HTML -> JSON (mark restored via parseHTML)
|
||||
const endJson = generateJSON(html(md), extensions);
|
||||
expect(countSpoilerMarks(endJson)).toBe(1);
|
||||
// The visible text survives.
|
||||
expect(JSON.stringify(endJson)).toContain("hidden");
|
||||
});
|
||||
|
||||
it("keeps the spoiler intact when it intersects a bold mark", () => {
|
||||
const startDoc = docWith({
|
||||
type: "text",
|
||||
text: "secret",
|
||||
marks: [{ type: "bold" }, { type: "spoiler" }],
|
||||
});
|
||||
|
||||
const md = htmlToMarkdown(generateHTML(startDoc, extensions));
|
||||
expect(md).toContain("data-spoiler=\"true\"");
|
||||
|
||||
const endJson = generateJSON(html(md), extensions);
|
||||
expect(countSpoilerMarks(endJson)).toBe(1);
|
||||
// Bold survives alongside the spoiler.
|
||||
expect(JSON.stringify(endJson)).toContain('"bold"');
|
||||
});
|
||||
});
|
||||
@@ -1,77 +1,147 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { htmlToMarkdown } from "./turndown.utils";
|
||||
import { markdownToHtml } from "./marked.utils";
|
||||
|
||||
/**
|
||||
* #206 mdrt-2 — Markdown export must never SILENTLY drop a block.
|
||||
* #206 mdrt-2 — Markdown export must never SILENTLY drop a block. (FIXED)
|
||||
*
|
||||
* `htmlToMarkdown` (turndown) only registers rules for a fixed set of custom
|
||||
* nodes (callout, taskItem, details, math, iframe, htmlEmbed, image, video,
|
||||
* footnote). Any other custom node — `transclusionReference`, `pageBreak`,
|
||||
* `mention`, `status` — falls through to turndown's default handling: an empty
|
||||
* wrapper is "blank" and removed, so the block disappears from the exported
|
||||
* Markdown with no trace. The invariant "never silently lose a block" is broken.
|
||||
* `htmlToMarkdown` (turndown) historically only registered rules for a fixed
|
||||
* set of custom nodes (callout, taskItem, details, math, iframe, htmlEmbed,
|
||||
* image, video, footnote). Any other custom node — `transclusionReference`,
|
||||
* `pageBreak`, `mention`, `status` — fell through to turndown's default
|
||||
* handling: an empty wrapper is "blank" and removed, so the block disappeared
|
||||
* from the exported Markdown with no trace, and `mention`/`status` collapsed to
|
||||
* bare text, losing their identity (data-id / data-color). The invariant
|
||||
* "never silently lose a block" was broken.
|
||||
*
|
||||
* The `it.fails` cases assert the DESIRED contract (the block survives export in
|
||||
* SOME form) and are RED today: they document the unfixed data loss and flip to
|
||||
* green the moment a turndown rule (real syntax or a lossless HTML-comment
|
||||
* placeholder) is added. A normal characterization `it` pins the exact current
|
||||
* lossy output so the regression is unambiguous.
|
||||
* The fix adds lossless turndown rules that re-emit each of these nodes as raw
|
||||
* HTML carrying every `data-*` attribute. Plain-Markdown viewers ignore the
|
||||
* inert tag; the import path round-trips it (`markdownToHtml` passes the raw
|
||||
* HTML through and each node's `parseHTML` rebuilds the ProseMirror node). These
|
||||
* tests assert the surviving contract (the block is preserved AND its identity
|
||||
* round-trips back through import).
|
||||
*/
|
||||
describe("htmlToMarkdown — custom nodes without a turndown rule (#206 mdrt-2)", () => {
|
||||
const wrap = (inner: string) =>
|
||||
`<p>before</p>${inner}<p>after</p>`;
|
||||
describe("htmlToMarkdown — custom nodes are preserved losslessly (#206 mdrt-2)", () => {
|
||||
const wrap = (inner: string) => `<p>before</p>${inner}<p>after</p>`;
|
||||
|
||||
it("CURRENTLY drops a pageBreak entirely (data loss)", () => {
|
||||
it("preserves a pageBreak block on Markdown export", () => {
|
||||
const md = htmlToMarkdown(
|
||||
wrap('<div data-type="pageBreak" class="page-break"></div>'),
|
||||
);
|
||||
// The page break vanishes: only the two paragraphs remain, nothing between.
|
||||
expect(md).toContain("before");
|
||||
expect(md).toContain("after");
|
||||
expect(md).not.toMatch(/page-?break/i);
|
||||
expect(md).not.toContain("---"); // not even a horizontal-rule fallback
|
||||
// The break survives as an inert raw-HTML tag, not silently dropped.
|
||||
expect(md).toMatch(/data-type="pageBreak"/);
|
||||
expect(md).toMatch(/page-?break/i);
|
||||
});
|
||||
|
||||
it("CURRENTLY drops a transclusionReference entirely (data loss)", () => {
|
||||
it("preserves a transclusionReference's identity on Markdown export", () => {
|
||||
const md = htmlToMarkdown(
|
||||
wrap('<div data-type="transclusionReference" data-id="abc"></div>'),
|
||||
);
|
||||
expect(md).toContain("before");
|
||||
expect(md).toContain("after");
|
||||
// The data-id (the only thing that gives the reference identity) is gone.
|
||||
expect(md).not.toContain("abc");
|
||||
// The data-id (the only thing that gives the reference identity) survives.
|
||||
expect(md).toContain("abc");
|
||||
expect(md).toMatch(/data-type="transclusionReference"/);
|
||||
});
|
||||
|
||||
it.fails(
|
||||
"should NOT lose a pageBreak block on Markdown export",
|
||||
() => {
|
||||
it("preserves a mention's data-id (stable identity) on Markdown export", () => {
|
||||
const md = htmlToMarkdown(
|
||||
'<p>hi <span data-type="mention" data-id="u1" data-label="Bob">@Bob</span> there</p>',
|
||||
);
|
||||
// The mention keeps its stable identity (data-id), not just the text.
|
||||
expect(md).toContain("u1");
|
||||
expect(md).toContain("Bob");
|
||||
expect(md).toMatch(/data-type="mention"/);
|
||||
});
|
||||
|
||||
it("preserves a status chip's color on Markdown export", () => {
|
||||
const md = htmlToMarkdown(
|
||||
'<p>s <span data-type="status" data-color="green">Done</span></p>',
|
||||
);
|
||||
// The chip's color (its identity) survives, not just the visible text.
|
||||
expect(md).toContain("green");
|
||||
expect(md).toContain("Done");
|
||||
expect(md).toMatch(/data-type="status"/);
|
||||
});
|
||||
|
||||
// The export form is only lossless if the import path can rebuild it. These
|
||||
// assert the full MD -> HTML round-trip restores the node + its attributes,
|
||||
// which is the marker <-> node contract each `parseHTML` relies on.
|
||||
describe("import round-trip (markdownToHtml restores the node)", () => {
|
||||
it("round-trips a pageBreak through export + import", async () => {
|
||||
const md = htmlToMarkdown(
|
||||
wrap('<div data-type="pageBreak" class="page-break"></div>'),
|
||||
);
|
||||
// Desired: the break survives in some form (e.g. a `---` rule or marker).
|
||||
expect(md).toMatch(/(-{3,}|page-?break)/i);
|
||||
},
|
||||
);
|
||||
const html = await markdownToHtml(md);
|
||||
expect(html).toMatch(/<div[^>]*data-type="pageBreak"[^>]*>/);
|
||||
expect(html).toContain("before");
|
||||
expect(html).toContain("after");
|
||||
});
|
||||
|
||||
it.fails(
|
||||
"should NOT lose a transclusionReference's identity on Markdown export",
|
||||
() => {
|
||||
it("round-trips a transclusionReference (keeps data-id)", async () => {
|
||||
const md = htmlToMarkdown(
|
||||
wrap('<div data-type="transclusionReference" data-id="abc"></div>'),
|
||||
);
|
||||
// Desired: the referenced id survives so the block can be rebuilt.
|
||||
expect(md).toContain("abc");
|
||||
},
|
||||
);
|
||||
const html = await markdownToHtml(md);
|
||||
expect(html).toMatch(/<div[^>]*data-type="transclusionReference"[^>]*>/);
|
||||
expect(html).toContain("abc");
|
||||
});
|
||||
|
||||
it.fails(
|
||||
"should NOT lose a mention's data-id on Markdown export",
|
||||
() => {
|
||||
it("round-trips a mention (keeps data-id + data-label)", async () => {
|
||||
const md = htmlToMarkdown(
|
||||
'<p>hi <span data-type="mention" data-id="u1" data-label="Bob">@Bob</span> there</p>',
|
||||
);
|
||||
// Desired: the mention keeps its stable identity (data-id), not just text.
|
||||
expect(md).toContain("u1");
|
||||
},
|
||||
);
|
||||
const html = await markdownToHtml(md);
|
||||
expect(html).toMatch(/<span[^>]*data-type="mention"[^>]*>/);
|
||||
expect(html).toContain("u1");
|
||||
expect(html).toContain("Bob");
|
||||
});
|
||||
|
||||
it("round-trips a status chip (keeps data-color)", async () => {
|
||||
const md = htmlToMarkdown(
|
||||
'<p>s <span data-type="status" data-color="green">Done</span></p>',
|
||||
);
|
||||
const html = await markdownToHtml(md);
|
||||
expect(html).toMatch(/<span[^>]*data-type="status"[^>]*>/);
|
||||
expect(html).toContain("green");
|
||||
});
|
||||
|
||||
// HTML special chars in an attribute value or in a node's text must be
|
||||
// ESCAPED when re-emitted as raw HTML, otherwise the exported tag is
|
||||
// malformed and `markdownToHtml`'s parser cannot restore the original value
|
||||
// (the same silent data loss this PR fixes). Dropping `<`/`>` escaping is the
|
||||
// dangerous regression: a stray `<` or `>` corrupts the tag (or injects new
|
||||
// markup), so the test data carries ALL of `&`, `"`, `<`, `>` in BOTH the
|
||||
// data-label attribute and the visible text. That fully exercises
|
||||
// escapeHtmlAttr's `&,",<,>` branches and escapeHtmlText's `&,<,>` branches
|
||||
// (escapeHtmlText leaves `"` literal); the alphanumeric-only cases above hit
|
||||
// none of them.
|
||||
it("escapes HTML special chars (& \" < >) in attrs + text and round-trips them", async () => {
|
||||
const md = htmlToMarkdown(
|
||||
`<p>hi <span data-type="mention" data-id="u1" data-label="A & <B> "C"">@A & <B> "C"</span> there</p>`,
|
||||
);
|
||||
|
||||
// (a) The exported Markdown carries a WELL-FORMED, correctly-escaped tag:
|
||||
// the attribute escapes `&`, `<`, `>` AND `"`; the text escapes `&`, `<`,
|
||||
// `>` (a `"` inside text content is legal, so it stays literal).
|
||||
expect(md).toContain('data-label="A & <B> "C""');
|
||||
expect(md).toContain('>@A & <B> "C"</span>');
|
||||
// And explicitly NOT the raw, tag-corrupting forms: a literal `<B>` (would
|
||||
// mean `<`/`>` escaping was dropped in either the attr or the text)...
|
||||
expect(md).not.toContain("<B>");
|
||||
// ...nor the malformed attribute that an unescaped `"` would produce.
|
||||
expect(md).not.toContain('data-label="A & <B> "C""');
|
||||
|
||||
// (b) Import restores the ORIGINAL (unescaped) values, attribute and text.
|
||||
const html = await markdownToHtml(md);
|
||||
const dom = new DOMParser().parseFromString(html as string, "text/html");
|
||||
const span = dom.querySelector('span[data-type="mention"]');
|
||||
expect(span).not.toBeNull();
|
||||
expect(span!.getAttribute("data-id")).toBe("u1");
|
||||
expect(span!.getAttribute("data-label")).toBe('A & <B> "C"');
|
||||
expect(span!.textContent).toBe('@A & <B> "C"');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -43,6 +43,54 @@ function fillEmptyFootnoteRefs(html: string): string {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* `pageBreak` and `transclusionReference` are childless atom <div>s. Like an
|
||||
* empty footnote ref (see above), turndown treats a childless block as "blank"
|
||||
* and replaces it with the blankRule BEFORE any custom rule can fire — so the
|
||||
* node disappears from the export with no trace (#206 mdrt-2). Inject a
|
||||
* zero-width space so the node is non-blank and our lossless rule runs; the
|
||||
* rule rebuilds the tag from the element's attributes, so the injected char
|
||||
* never reaches the output.
|
||||
*/
|
||||
function fillEmptyAtomBlocks(html: string): string {
|
||||
return html.replace(
|
||||
/<div\b([^>]*\bdata-type="(?:pageBreak|transclusionReference)"[^>]*)>\s*<\/div>/gi,
|
||||
(_m, attrs) => `<div${attrs}></div>`,
|
||||
);
|
||||
}
|
||||
|
||||
/** HTML-escape an attribute value so a re-emitted raw-HTML tag is well-formed. */
|
||||
function escapeHtmlAttr(value: string): string {
|
||||
return value
|
||||
.replace(/&/g, '&')
|
||||
.replace(/"/g, '"')
|
||||
.replace(/</g, '<')
|
||||
.replace(/>/g, '>');
|
||||
}
|
||||
|
||||
/** HTML-escape text placed inside a re-emitted raw-HTML element. */
|
||||
function escapeHtmlText(value: string): string {
|
||||
return value
|
||||
.replace(/&/g, '&')
|
||||
.replace(/</g, '<')
|
||||
.replace(/>/g, '>');
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize ALL of an element's attributes back to a raw-HTML attribute string
|
||||
* (leading space included). Generic on purpose: a custom node's identity lives
|
||||
* entirely in its `data-*` attributes (data-id, data-color, data-source-page-id,
|
||||
* data-transclusion-id, …), and serializing every attribute keeps the export
|
||||
* lossless regardless of which attributes a given node carries.
|
||||
*/
|
||||
function serializeAttrs(node: any): string {
|
||||
const attrs = node?.attributes;
|
||||
if (!attrs) return '';
|
||||
return Array.from(attrs as ArrayLike<{ name: string; value: string }>)
|
||||
.map((attr) => ` ${attr.name}="${escapeHtmlAttr(attr.value ?? '')}"`)
|
||||
.join('');
|
||||
}
|
||||
|
||||
export function htmlToMarkdown(html: string): string {
|
||||
const turndownService = new TurndownService({
|
||||
headingStyle: 'atx',
|
||||
@@ -65,16 +113,88 @@ export function htmlToMarkdown(html: string): string {
|
||||
mathBlock,
|
||||
iframeEmbed,
|
||||
htmlEmbed,
|
||||
spoiler,
|
||||
image,
|
||||
video,
|
||||
footnoteReference,
|
||||
footnotesList,
|
||||
pageBreak,
|
||||
transclusionReference,
|
||||
mention,
|
||||
status,
|
||||
]);
|
||||
return turndownService
|
||||
.turndown(fillEmptyFootnoteRefs(html))
|
||||
.turndown(fillEmptyAtomBlocks(fillEmptyFootnoteRefs(html)))
|
||||
.replaceAll('<br>', ' ');
|
||||
}
|
||||
|
||||
/**
|
||||
* Lossless export rules for custom nodes that have NO native Markdown syntax
|
||||
* (#206 mdrt-2). Markdown cannot represent a page break, a transclusion
|
||||
* reference, a mention's stable id, or a status chip's color — so rather than
|
||||
* letting turndown silently drop them, each rule re-emits the node as raw HTML
|
||||
* carrying every `data-*` attribute. Plain-Markdown viewers ignore the inert
|
||||
* tag, and the import path round-trips it: `markdownToHtml` passes raw HTML
|
||||
* through and each node's `parseHTML` (`div[data-type="…"]`, `span[…]`) rebuilds
|
||||
* the ProseMirror node with its attributes intact.
|
||||
*/
|
||||
function pageBreak(turndownService: _TurndownService) {
|
||||
turndownService.addRule('pageBreak', {
|
||||
filter: function (node: HTMLInputElement) {
|
||||
return (
|
||||
node.nodeName === 'DIV' &&
|
||||
node.getAttribute('data-type') === 'pageBreak'
|
||||
);
|
||||
},
|
||||
replacement: function (_content: string, node: HTMLInputElement) {
|
||||
return `\n\n<div${serializeAttrs(node)}></div>\n\n`;
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function transclusionReference(turndownService: _TurndownService) {
|
||||
turndownService.addRule('transclusionReference', {
|
||||
filter: function (node: HTMLInputElement) {
|
||||
return (
|
||||
node.nodeName === 'DIV' &&
|
||||
node.getAttribute('data-type') === 'transclusionReference'
|
||||
);
|
||||
},
|
||||
replacement: function (_content: string, node: HTMLInputElement) {
|
||||
return `\n\n<div${serializeAttrs(node)}></div>\n\n`;
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function mention(turndownService: _TurndownService) {
|
||||
turndownService.addRule('mention', {
|
||||
filter: function (node: HTMLInputElement) {
|
||||
return (
|
||||
node.nodeName === 'SPAN' &&
|
||||
node.getAttribute('data-type') === 'mention'
|
||||
);
|
||||
},
|
||||
replacement: function (_content: string, node: HTMLInputElement) {
|
||||
const text = escapeHtmlText(node.textContent || '');
|
||||
return `<span${serializeAttrs(node)}>${text}</span>`;
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function status(turndownService: _TurndownService) {
|
||||
turndownService.addRule('status', {
|
||||
filter: function (node: HTMLInputElement) {
|
||||
return (
|
||||
node.nodeName === 'SPAN' && node.getAttribute('data-type') === 'status'
|
||||
);
|
||||
},
|
||||
replacement: function (_content: string, node: HTMLInputElement) {
|
||||
const text = escapeHtmlText(node.textContent || '');
|
||||
return `<span${serializeAttrs(node)}>${text}</span>`;
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize the `htmlEmbed` node to Markdown.
|
||||
*
|
||||
@@ -101,6 +221,29 @@ function htmlEmbed(turndownService: _TurndownService) {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize the `spoiler` inline mark to lossless raw inline HTML.
|
||||
*
|
||||
* Markdown has no native spoiler syntax, so we emit the same `<span
|
||||
* data-spoiler="true">…</span>` the mark renders. `marked` passes inline raw HTML
|
||||
* through untouched, and `generateJSON` restores the mark via its parseHTML, so
|
||||
* the round-trip MD -> HTML -> JSON keeps the spoiler intact. The UI-only
|
||||
* `is-revealed` state is never serialized.
|
||||
*/
|
||||
function spoiler(turndownService: _TurndownService) {
|
||||
turndownService.addRule('spoiler', {
|
||||
filter: function (node: HTMLInputElement) {
|
||||
return (
|
||||
node.nodeName === 'SPAN' &&
|
||||
node.getAttribute('data-spoiler') === 'true'
|
||||
);
|
||||
},
|
||||
replacement: function (content: string) {
|
||||
return `<span data-spoiler="true">${content}</span>`;
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function listParagraph(turndownService: _TurndownService) {
|
||||
turndownService.addRule('paragraph', {
|
||||
filter: ['p'],
|
||||
@@ -258,6 +401,17 @@ function image(turndownService: _TurndownService) {
|
||||
replacement: function (_content: string, node: HTMLInputElement) {
|
||||
const src = node.getAttribute('src') || '';
|
||||
if (!src) return '';
|
||||
const caption = node.getAttribute('data-caption') || '';
|
||||
if (caption) {
|
||||
// ![]() can't carry a caption, so emit a raw <img> wrapped in a block
|
||||
// <div>. marked passes it through and the image extension's parseHTML
|
||||
// restores the caption from data-caption.
|
||||
const parts = [`src="${escapeHtmlAttr(src)}"`];
|
||||
const alt = node.getAttribute('alt') || '';
|
||||
if (alt) parts.push(`alt="${escapeHtmlAttr(alt)}"`);
|
||||
parts.push(`data-caption="${escapeHtmlAttr(caption)}"`);
|
||||
return `<div><img ${parts.join(' ')}></div>`;
|
||||
}
|
||||
const alt = sanitizeMdLinkText(node.getAttribute('alt') || '');
|
||||
const title = node.getAttribute('title') || '';
|
||||
const titlePart = title ? ' "' + title.replace(/"/g, '\\"') + '"' : '';
|
||||
|
||||
74
packages/editor-ext/src/lib/spoiler/spoiler.ts
Normal file
74
packages/editor-ext/src/lib/spoiler/spoiler.ts
Normal file
@@ -0,0 +1,74 @@
|
||||
import { Mark, markInputRule, mergeAttributes } from "@tiptap/core";
|
||||
|
||||
export interface SpoilerOptions {
|
||||
HTMLAttributes: Record<string, unknown>;
|
||||
}
|
||||
|
||||
declare module "@tiptap/core" {
|
||||
interface Commands<ReturnType> {
|
||||
spoiler: {
|
||||
setSpoiler: () => ReturnType;
|
||||
toggleSpoiler: () => ReturnType;
|
||||
unsetSpoiler: () => ReturnType;
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Discord-style `||text||` input rule. Requires a non-space right after the
|
||||
// opening `||` and a non-space right before the closing `||` so empty/padded
|
||||
// markers don't match.
|
||||
const inputRegex = /(?:^|\s)(\|\|(?!\s)([^|]+)(?<!\s)\|\|)$/;
|
||||
|
||||
export const Spoiler = Mark.create<SpoilerOptions>({
|
||||
name: "spoiler",
|
||||
|
||||
// Don't bleed onto text typed at the boundary (mirrors link).
|
||||
inclusive: false,
|
||||
|
||||
addOptions() {
|
||||
return {
|
||||
HTMLAttributes: {},
|
||||
};
|
||||
},
|
||||
|
||||
parseHTML() {
|
||||
return [{ tag: "span[data-spoiler]" }];
|
||||
},
|
||||
|
||||
renderHTML({ HTMLAttributes }) {
|
||||
return [
|
||||
"span",
|
||||
mergeAttributes(this.options.HTMLAttributes, HTMLAttributes, {
|
||||
"data-spoiler": "true",
|
||||
class: "spoiler",
|
||||
}),
|
||||
0,
|
||||
];
|
||||
},
|
||||
|
||||
addCommands() {
|
||||
return {
|
||||
setSpoiler:
|
||||
() =>
|
||||
({ commands }) =>
|
||||
commands.setMark(this.name),
|
||||
toggleSpoiler:
|
||||
() =>
|
||||
({ commands }) =>
|
||||
commands.toggleMark(this.name),
|
||||
unsetSpoiler:
|
||||
() =>
|
||||
({ commands }) =>
|
||||
commands.unsetMark(this.name),
|
||||
};
|
||||
},
|
||||
|
||||
addInputRules() {
|
||||
return [markInputRule({ find: inputRegex, type: this.type })];
|
||||
},
|
||||
|
||||
// No addKeyboardShortcuts: the issue's proposed `Mod-Shift-s` is already taken
|
||||
// by the built-in Strike mark (and `Mod-Shift-h` by Highlight). The `||text||`
|
||||
// input rule plus the bubble-menu button cover ergonomics, so we omit a hotkey
|
||||
// rather than collide with an existing one.
|
||||
});
|
||||
@@ -16,7 +16,7 @@ license.
|
||||
> that interface. Other Docmost MCPs are human-shaped — they expose "open the page" and
|
||||
> "replace the page"; this one exposes the editing primitives a model is good at.
|
||||
|
||||
It exposes **40 tools** built around three ideas that the other Docmost MCPs do not
|
||||
It exposes **41 tools** built around three ideas that the other Docmost MCPs do not
|
||||
combine:
|
||||
|
||||
1. **Surgical, token-cheap edits.** Address a single block by id and patch it, or run
|
||||
@@ -106,7 +106,7 @@ There are several Docmost MCPs. Here is a capability-by-capability comparison.
|
||||
|
||||
## Tools
|
||||
|
||||
All 40 tools, grouped by what you'd reach for them.
|
||||
All 41 tools, grouped by what you'd reach for them.
|
||||
|
||||
### Exploration & retrieval
|
||||
|
||||
@@ -219,6 +219,8 @@ All 40 tools, grouped by what you'd reach for them.
|
||||
- **`list_comments`** — List a page's comments (content returned as Markdown).
|
||||
- **`update_comment`** — Edit an existing comment.
|
||||
- **`delete_comment`** — Delete a comment.
|
||||
- **`resolve_comment`** — Resolve (close) or reopen a comment thread (reversible). Only top-level
|
||||
comments can be resolved; the thread and its replies are kept, unlike `delete_comment`.
|
||||
- **`check_new_comments`** — Find comments created after a given ISO-8601 timestamp across
|
||||
a space, optionally scoped to a page subtree — ideal for an agent that watches a doc for
|
||||
feedback.
|
||||
@@ -262,7 +264,7 @@ so capable clients steer the model automatically.
|
||||
- **Reads**: `get_page` (Markdown) / `get_page_json` (lossless ProseMirror with ids).
|
||||
- **Review changes**: `list_page_history` → `diff_page_versions` → `restore_page_version`.
|
||||
- **Comments**: `create_comment` (with optional inline anchoring) / `list_comments` /
|
||||
`update_comment` / `delete_comment` / `check_new_comments`.
|
||||
`update_comment` / `resolve_comment` / `delete_comment` / `check_new_comments`.
|
||||
- **Navigate a page cheaply** (find a section/table, grab a block id): `get_outline` →
|
||||
`get_node`.
|
||||
- **Tables** (add/remove a row, set a cell): `table_get` / `table_insert_row` /
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
> «открыть страницу» и «заменить страницу»; этот даёт примитивы редактирования, в которых
|
||||
> модель сильна.
|
||||
|
||||
Сервер предоставляет **40 инструментов**, построенных вокруг трёх идей, которые другие
|
||||
Сервер предоставляет **41 инструмент**, построенный вокруг трёх идей, которые другие
|
||||
Docmost-MCP не сочетают:
|
||||
|
||||
1. **Точечные, экономичные по токенам правки.** Адресуйте отдельный блок по id и патчите
|
||||
@@ -109,7 +109,7 @@ Docmost-MCP не сочетают:
|
||||
|
||||
## Инструменты
|
||||
|
||||
Все 40 инструментов, сгруппированы по задачам, для которых вы их возьмёте.
|
||||
Все 41 инструмент, сгруппированы по задачам, для которых вы их возьмёте.
|
||||
|
||||
### Чтение и поиск
|
||||
|
||||
@@ -226,6 +226,8 @@ Docmost-MCP не сочетают:
|
||||
- **`list_comments`** — Список комментариев страницы (контент возвращается как Markdown).
|
||||
- **`update_comment`** — Изменить существующий комментарий.
|
||||
- **`delete_comment`** — Удалить комментарий.
|
||||
- **`resolve_comment`** — Закрыть (resolve) или переоткрыть тред комментария (обратимо). Resolve
|
||||
доступен только для корневых комментариев; тред и ответы сохраняются, в отличие от `delete_comment`.
|
||||
- **`check_new_comments`** — Найти комментарии, созданные после заданной метки времени
|
||||
ISO-8601, по пространству, опционально в рамках поддерева страниц — идеально для агента,
|
||||
который следит за обратной связью в документе.
|
||||
@@ -271,7 +273,7 @@ Docmost-MCP не сочетают:
|
||||
- **Просмотр изменений**: `list_page_history` → `diff_page_versions` →
|
||||
`restore_page_version`.
|
||||
- **Комментарии**: `create_comment` (с опциональной inline-привязкой) / `list_comments` /
|
||||
`update_comment` / `delete_comment` / `check_new_comments`.
|
||||
`update_comment` / `resolve_comment` / `delete_comment` / `check_new_comments`.
|
||||
- **Дешёвая навигация по странице** (найти раздел/таблицу, получить id блока): `get_outline`
|
||||
→ `get_node`.
|
||||
- **Таблицы** (добавить/удалить строку, задать ячейку): `table_get` / `table_insert_row` /
|
||||
|
||||
@@ -27,7 +27,7 @@ const VERSION = packageJson.version;
|
||||
// --- Modern McpServer Implementation ---
|
||||
// Editing guide surfaced to MCP clients in the initialize result so they can
|
||||
// pick the right tool by intent and avoid resending whole documents.
|
||||
const SERVER_INSTRUCTIONS = "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). 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). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " +
|
||||
const SERVER_INSTRUCTIONS = "Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). 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). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, resolve_comment (resolve/reopen a thread, reversible — prefer over delete to close), delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " +
|
||||
"Complex/scripted rewrite (multiple coordinated edits, footnotes, 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. " +
|
||||
"Review what changed -> diff_page_versions (compare a historyId to current, or two history versions). See a page's 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.";
|
||||
@@ -603,6 +603,27 @@ export function createDocmostMcpServer(config) {
|
||||
],
|
||||
};
|
||||
});
|
||||
// Tool: resolve_comment
|
||||
server.registerTool("resolve_comment", {
|
||||
description: "Resolve (close) or reopen a comment thread. Only top-level comments can " +
|
||||
"be resolved — the server rejects resolving a reply. Reversible: pass " +
|
||||
"resolved=false to reopen. Resolving keeps the thread and its replies " +
|
||||
"(unlike delete_comment, which permanently removes them).",
|
||||
inputSchema: {
|
||||
commentId: z
|
||||
.string()
|
||||
.min(1)
|
||||
.describe("ID of the top-level comment thread to resolve or reopen"),
|
||||
resolved: z
|
||||
.boolean()
|
||||
.optional()
|
||||
.default(true)
|
||||
.describe("true (default) marks the thread resolved/closed; false reopens it"),
|
||||
},
|
||||
}, async ({ commentId, resolved }) => {
|
||||
const result = await docmostClient.resolveComment(commentId, resolved);
|
||||
return jsonContent(result);
|
||||
});
|
||||
// Tool: check_new_comments
|
||||
server.registerTool("check_new_comments", {
|
||||
description: "Check for new comments across pages in a space since a given timestamp. " +
|
||||
|
||||
@@ -271,6 +271,25 @@ const TextStyle = Mark.create({
|
||||
return ["span", HTMLAttributes, 0];
|
||||
},
|
||||
});
|
||||
/**
|
||||
* Inline spoiler mark. Mirrors the @docmost/editor-ext `spoiler` mark so a
|
||||
* document carrying a spoiler survives the MCP read -> transform -> write path
|
||||
* (and markdown export) instead of silently dropping the unrecognized mark.
|
||||
* packages/mcp does NOT depend on editor-ext, so the definition is kept local;
|
||||
* it parses span[data-spoiler] and renders the same span[data-spoiler][class]
|
||||
* the editor-ext mark emits.
|
||||
*/
|
||||
const Spoiler = Mark.create({
|
||||
name: "spoiler",
|
||||
// Don't bleed onto text typed at the boundary (mirrors editor-ext).
|
||||
inclusive: false,
|
||||
parseHTML() {
|
||||
return [{ tag: "span[data-spoiler]" }];
|
||||
},
|
||||
renderHTML({ HTMLAttributes }) {
|
||||
return ["span", { "data-spoiler": "true", class: "spoiler", ...HTMLAttributes }, 0];
|
||||
},
|
||||
});
|
||||
/**
|
||||
* Passthrough definitions for the remaining Docmost-specific nodes.
|
||||
*
|
||||
@@ -1070,7 +1089,24 @@ export const docmostExtensions = [
|
||||
heading: {},
|
||||
link: { openOnClick: false },
|
||||
}),
|
||||
Image.configure({ inline: false }),
|
||||
// Stock @tiptap/extension-image has no caption attribute, so a round-trip
|
||||
// through this schema would drop the data-caption the client TiptapImage
|
||||
// emits. Mirror editor-ext image.ts: add a caption attribute that parses
|
||||
// data-caption and re-renders it only when set (caption-less images stay
|
||||
// clean), keeping the MCP markdown round-trip lossless.
|
||||
Image.extend({
|
||||
addAttributes() {
|
||||
const parent = this.parent?.() ?? {};
|
||||
return {
|
||||
...parent,
|
||||
caption: {
|
||||
default: undefined,
|
||||
parseHTML: (el) => el.getAttribute("data-caption") || undefined,
|
||||
renderHTML: (attrs) => attrs.caption ? { "data-caption": attrs.caption } : {},
|
||||
},
|
||||
};
|
||||
},
|
||||
}).configure({ inline: false }),
|
||||
TaskList,
|
||||
TaskItem.configure({ nested: true }),
|
||||
// Highlight stores its color unescaped and Docmost interpolates it into
|
||||
@@ -1097,6 +1133,7 @@ export const docmostExtensions = [
|
||||
// generateJSON drops <span style="color: ...">, defeating the color import.
|
||||
TextStyle,
|
||||
Comment,
|
||||
Spoiler,
|
||||
Callout,
|
||||
Table,
|
||||
TableRow,
|
||||
|
||||
@@ -160,6 +160,12 @@ export function convertProseMirrorToMarkdown(content) {
|
||||
}
|
||||
break;
|
||||
}
|
||||
case "spoiler":
|
||||
// Markdown has no native spoiler syntax, so emit the same
|
||||
// lossless raw HTML the editor-ext turndown rule produces; the
|
||||
// schema's Spoiler mark parses span[data-spoiler] back on import.
|
||||
textContent = `<span data-spoiler="true">${textContent}</span>`;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -207,16 +213,27 @@ export function convertProseMirrorToMarkdown(content) {
|
||||
// Two trailing spaces before the newline encode a markdown hard break;
|
||||
// a bare "\n" would be reimported as a soft break and lost.
|
||||
return " \n";
|
||||
case "image":
|
||||
case "image": {
|
||||
const imgAlt = node.attrs?.alt || "";
|
||||
const imgCaption = node.attrs?.caption || "";
|
||||
if (imgCaption) {
|
||||
// ![]() can't carry a caption, so (symmetric to video) emit a raw
|
||||
// <img> wrapped in a block <div>. On import marked.parse keeps the raw
|
||||
// HTML and generateJSON runs the image extension's parseHTML, which
|
||||
// restores the caption from data-caption.
|
||||
const parts = [`src="${escapeAttr(node.attrs?.src ?? "")}"`];
|
||||
if (imgAlt)
|
||||
parts.push(`alt="${escapeAttr(imgAlt)}"`);
|
||||
parts.push(`data-caption="${escapeAttr(imgCaption)}"`);
|
||||
return `<div><img ${parts.join(" ")}></div>`;
|
||||
}
|
||||
// Neutralize characters that could break out of the markdown image
|
||||
// URL: spaces/newlines and parentheses would terminate the (...) target
|
||||
// and let a stored src inject following markdown/HTML. Percent-encode
|
||||
// them so the URL stays a single inert token.
|
||||
const imgSrc = encodeMdUrl(node.attrs?.src);
|
||||
// No "caption" attribute exists in the Docmost image schema, so we do
|
||||
// not emit one (the previous caption branch was dead).
|
||||
return ``;
|
||||
}
|
||||
case "video": {
|
||||
// Emit the schema-matching <video> element so generateJSON rebuilds the
|
||||
// node with its attrs intact. The schema's parseHTML reads src/aria-label
|
||||
@@ -618,6 +635,8 @@ export function convertProseMirrorToMarkdown(content) {
|
||||
const parts = [`src="${escapeAttr(attrs.src ?? "")}"`];
|
||||
if (attrs.alt)
|
||||
parts.push(`alt="${escapeAttr(attrs.alt)}"`);
|
||||
if (attrs.caption)
|
||||
parts.push(`data-caption="${escapeAttr(attrs.caption)}"`);
|
||||
if (attrs.title)
|
||||
parts.push(`title="${escapeAttr(attrs.title)}"`);
|
||||
if (attrs.width != null)
|
||||
|
||||
@@ -38,7 +38,7 @@ const VERSION = packageJson.version;
|
||||
// Editing guide surfaced to MCP clients in the initialize result so they can
|
||||
// pick the right tool by intent and avoid resending whole documents.
|
||||
const SERVER_INSTRUCTIONS =
|
||||
"Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). 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). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " +
|
||||
"Docmost editing guide — choose the tool by intent: fix wording/typos/numbers (text inside blocks) -> edit_page_text (no node id needed). Change ONE block (paragraph/heading/callout/table cell/etc.) structurally -> patch_node (address by attrs.id from get_page_json). 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). Images -> insert_image (add an image from a web URL) / replace_image (swap an existing image for one from a web URL). New page -> create_page (Markdown). Bulk/structural rewrite or nodes without an id -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Rename a page (title only) -> rename_page. Read -> get_page (Markdown, lossy) or get_page_json (lossless ProseMirror with block ids). Comments -> create_comment (always inline; requires an EXACT selection — the contiguous text to anchor/highlight on; fails rather than leaving an unanchored comment), list_comments, update_comment, resolve_comment (resolve/reopen a thread, reversible — prefer over delete to close), delete_comment, check_new_comments. Tip: read block ids via get_page_json, then use patch_node/insert_node/delete_node so you never resend the full document. " +
|
||||
"Complex/scripted rewrite (multiple coordinated edits, footnotes, 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. " +
|
||||
"Review what changed -> diff_page_versions (compare a historyId to current, or two history versions). See a page's 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.";
|
||||
@@ -838,6 +838,35 @@ server.registerTool(
|
||||
},
|
||||
);
|
||||
|
||||
// Tool: resolve_comment
|
||||
server.registerTool(
|
||||
"resolve_comment",
|
||||
{
|
||||
description:
|
||||
"Resolve (close) or reopen a comment thread. Only top-level comments can " +
|
||||
"be resolved — the server rejects resolving a reply. Reversible: pass " +
|
||||
"resolved=false to reopen. Resolving keeps the thread and its replies " +
|
||||
"(unlike delete_comment, which permanently removes them).",
|
||||
inputSchema: {
|
||||
commentId: z
|
||||
.string()
|
||||
.min(1)
|
||||
.describe("ID of the top-level comment thread to resolve or reopen"),
|
||||
resolved: z
|
||||
.boolean()
|
||||
.optional()
|
||||
.default(true)
|
||||
.describe(
|
||||
"true (default) marks the thread resolved/closed; false reopens it",
|
||||
),
|
||||
},
|
||||
},
|
||||
async ({ commentId, resolved }) => {
|
||||
const result = await docmostClient.resolveComment(commentId, resolved);
|
||||
return jsonContent(result);
|
||||
},
|
||||
);
|
||||
|
||||
// Tool: check_new_comments
|
||||
server.registerTool(
|
||||
"check_new_comments",
|
||||
|
||||
@@ -298,6 +298,26 @@ const TextStyle = Mark.create({
|
||||
},
|
||||
});
|
||||
|
||||
/**
|
||||
* Inline spoiler mark. Mirrors the @docmost/editor-ext `spoiler` mark so a
|
||||
* document carrying a spoiler survives the MCP read -> transform -> write path
|
||||
* (and markdown export) instead of silently dropping the unrecognized mark.
|
||||
* packages/mcp does NOT depend on editor-ext, so the definition is kept local;
|
||||
* it parses span[data-spoiler] and renders the same span[data-spoiler][class]
|
||||
* the editor-ext mark emits.
|
||||
*/
|
||||
const Spoiler = Mark.create({
|
||||
name: "spoiler",
|
||||
// Don't bleed onto text typed at the boundary (mirrors editor-ext).
|
||||
inclusive: false,
|
||||
parseHTML() {
|
||||
return [{ tag: "span[data-spoiler]" }];
|
||||
},
|
||||
renderHTML({ HTMLAttributes }) {
|
||||
return ["span", { "data-spoiler": "true", class: "spoiler", ...HTMLAttributes }, 0];
|
||||
},
|
||||
});
|
||||
|
||||
/**
|
||||
* Passthrough definitions for the remaining Docmost-specific nodes.
|
||||
*
|
||||
@@ -1164,7 +1184,26 @@ export const docmostExtensions = [
|
||||
heading: {},
|
||||
link: { openOnClick: false },
|
||||
}),
|
||||
Image.configure({ inline: false }),
|
||||
// Stock @tiptap/extension-image has no caption attribute, so a round-trip
|
||||
// through this schema would drop the data-caption the client TiptapImage
|
||||
// emits. Mirror editor-ext image.ts: add a caption attribute that parses
|
||||
// data-caption and re-renders it only when set (caption-less images stay
|
||||
// clean), keeping the MCP markdown round-trip lossless.
|
||||
Image.extend({
|
||||
addAttributes() {
|
||||
const parent = this.parent?.() ?? {};
|
||||
return {
|
||||
...parent,
|
||||
caption: {
|
||||
default: undefined,
|
||||
parseHTML: (el: HTMLElement) =>
|
||||
el.getAttribute("data-caption") || undefined,
|
||||
renderHTML: (attrs: Record<string, any>) =>
|
||||
attrs.caption ? { "data-caption": attrs.caption } : {},
|
||||
},
|
||||
};
|
||||
},
|
||||
}).configure({ inline: false }),
|
||||
TaskList,
|
||||
TaskItem.configure({ nested: true }),
|
||||
// Highlight stores its color unescaped and Docmost interpolates it into
|
||||
@@ -1194,6 +1233,7 @@ export const docmostExtensions = [
|
||||
// generateJSON drops <span style="color: ...">, defeating the color import.
|
||||
TextStyle,
|
||||
Comment,
|
||||
Spoiler,
|
||||
Callout,
|
||||
Table,
|
||||
TableRow,
|
||||
|
||||
@@ -167,6 +167,12 @@ export function convertProseMirrorToMarkdown(content: any): string {
|
||||
}
|
||||
break;
|
||||
}
|
||||
case "spoiler":
|
||||
// Markdown has no native spoiler syntax, so emit the same
|
||||
// lossless raw HTML the editor-ext turndown rule produces; the
|
||||
// schema's Spoiler mark parses span[data-spoiler] back on import.
|
||||
textContent = `<span data-spoiler="true">${textContent}</span>`;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -228,16 +234,26 @@ export function convertProseMirrorToMarkdown(content: any): string {
|
||||
// a bare "\n" would be reimported as a soft break and lost.
|
||||
return " \n";
|
||||
|
||||
case "image":
|
||||
case "image": {
|
||||
const imgAlt = node.attrs?.alt || "";
|
||||
const imgCaption = node.attrs?.caption || "";
|
||||
if (imgCaption) {
|
||||
// ![]() can't carry a caption, so (symmetric to video) emit a raw
|
||||
// <img> wrapped in a block <div>. On import marked.parse keeps the raw
|
||||
// HTML and generateJSON runs the image extension's parseHTML, which
|
||||
// restores the caption from data-caption.
|
||||
const parts: string[] = [`src="${escapeAttr(node.attrs?.src ?? "")}"`];
|
||||
if (imgAlt) parts.push(`alt="${escapeAttr(imgAlt)}"`);
|
||||
parts.push(`data-caption="${escapeAttr(imgCaption)}"`);
|
||||
return `<div><img ${parts.join(" ")}></div>`;
|
||||
}
|
||||
// Neutralize characters that could break out of the markdown image
|
||||
// URL: spaces/newlines and parentheses would terminate the (...) target
|
||||
// and let a stored src inject following markdown/HTML. Percent-encode
|
||||
// them so the URL stays a single inert token.
|
||||
const imgSrc = encodeMdUrl(node.attrs?.src);
|
||||
// No "caption" attribute exists in the Docmost image schema, so we do
|
||||
// not emit one (the previous caption branch was dead).
|
||||
return ``;
|
||||
}
|
||||
|
||||
case "video": {
|
||||
// Emit the schema-matching <video> element so generateJSON rebuilds the
|
||||
@@ -678,6 +694,8 @@ export function convertProseMirrorToMarkdown(content: any): string {
|
||||
const attrs = node.attrs || {};
|
||||
const parts: string[] = [`src="${escapeAttr(attrs.src ?? "")}"`];
|
||||
if (attrs.alt) parts.push(`alt="${escapeAttr(attrs.alt)}"`);
|
||||
if (attrs.caption)
|
||||
parts.push(`data-caption="${escapeAttr(attrs.caption)}"`);
|
||||
if (attrs.title) parts.push(`title="${escapeAttr(attrs.title)}"`);
|
||||
if (attrs.width != null) parts.push(`width="${escapeAttr(attrs.width)}"`);
|
||||
if (attrs.height != null) parts.push(`height="${escapeAttr(attrs.height)}"`);
|
||||
|
||||
@@ -469,6 +469,17 @@ async function main() {
|
||||
check("update_comment + get_comment: content updated", got.data.content.includes("Обновлённый"), got.data.content);
|
||||
const news = await client.checkNewComments(spaceId, beforeComments, pageId);
|
||||
check("check_new_comments: finds new comments in subtree", news.totalNewComments >= 2, `total=${news.totalNewComments}`);
|
||||
// resolve_comment: close the top-level thread, verify resolvedAt surfaces, then reopen
|
||||
const resolvedRes = await client.resolveComment(c1.data.id, true);
|
||||
check("resolve_comment: marks resolved", resolvedRes.success === true && resolvedRes.resolved === true);
|
||||
const listResolved = await client.listComments(pageId);
|
||||
const c1Resolved = listResolved.find((c) => c.id === c1.data.id);
|
||||
check("resolve_comment: resolvedAt set in list", !!c1Resolved?.resolvedAt, `resolvedAt=${c1Resolved?.resolvedAt}`);
|
||||
const reopenedRes = await client.resolveComment(c1.data.id, false);
|
||||
check("resolve_comment: reopen succeeds", reopenedRes.resolved === false);
|
||||
const listReopened = await client.listComments(pageId);
|
||||
const c1Reopened = listReopened.find((c) => c.id === c1.data.id);
|
||||
check("resolve_comment: resolvedAt cleared on reopen", !c1Reopened?.resolvedAt, `resolvedAt=${c1Reopened?.resolvedAt}`);
|
||||
await client.deleteComment(reply.data.id);
|
||||
await client.deleteComment(c1.data.id);
|
||||
const listAfter = await client.listComments(pageId);
|
||||
|
||||
213
packages/mcp/test/unit/client-host-contract.test.mjs
Normal file
213
packages/mcp/test/unit/client-host-contract.test.mjs
Normal file
@@ -0,0 +1,213 @@
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, resolve } from "node:path";
|
||||
|
||||
import { DocmostClient } from "../../build/index.js";
|
||||
|
||||
// Drift guard for the THIRD hand-written layer of the AI tool set (issue #193,
|
||||
// layer 3): the in-app server hand-mirrors the DocmostClient method signatures
|
||||
// it consumes as the `DocmostClientLike` interface in
|
||||
// apps/server/src/core/ai-chat/tools/docmost-client.loader.ts ("Signatures here
|
||||
// mirror that file exactly"). That mirror lives across the ESM(mcp)/CJS(server)
|
||||
// boundary and the package ships NO .d.ts, so the server typecheck cannot verify
|
||||
// the names against the real class — a rename/removal in client.ts would surface
|
||||
// only as a runtime "x is not a function" inside an agent tool call.
|
||||
//
|
||||
// SCOPE: this guard checks the method-NAME set only, not signatures. It pins the
|
||||
// contract from the mcp side (ESM, where the real class is directly importable):
|
||||
// every method the embedding host depends on MUST exist as a function on a real
|
||||
// DocmostClient instance. If you rename/remove a client method, this fails here
|
||||
// AND you must update DocmostClientLike to match. It does NOT verify parameter or
|
||||
// return-type parity — signature drift between the hand-mirror and client.ts can
|
||||
// still ship silently; full signature/type parity is the deferred staged-plan
|
||||
// item below.
|
||||
//
|
||||
// Keep the HOST_CONTRACT_METHODS NAME list aligned with the method NAMES declared
|
||||
// in the server's DocmostClientLike interface (the in-app per-user tool adapter
|
||||
// only — it is a SUBSET of the DocmostClient surface — covers only what the in-app adapter
|
||||
// consumes; the standalone MCP transport (packages/mcp/src/index.ts) calls additional
|
||||
// client methods (insertImage/replaceImage/deleteComment/updateComment/insertFootnote)
|
||||
// that this guard does NOT track — the MCP transport's own typecheck covers those). Full type-derivation
|
||||
// of DocmostClientLike from this class is deferred (see the staged plan in
|
||||
// docmost-client.loader.ts): the package emits no declarations and the real
|
||||
// (inferred, concrete) return types conflict with the host's loose
|
||||
// `Record<string,unknown>` + `as`-cast result handling.
|
||||
const HOST_CONTRACT_METHODS = [
|
||||
// read
|
||||
"search",
|
||||
"getPage",
|
||||
"getWorkspace",
|
||||
"getSpaces",
|
||||
"listPages",
|
||||
"listSidebarPages",
|
||||
"getOutline",
|
||||
"getPageJson",
|
||||
"getNode",
|
||||
"getTable",
|
||||
"listComments",
|
||||
"getComment",
|
||||
"checkNewComments",
|
||||
"listShares",
|
||||
"listPageHistory",
|
||||
"getPageHistory",
|
||||
"diffPageVersions",
|
||||
"exportPageMarkdown",
|
||||
// write (page)
|
||||
"createPage",
|
||||
"updatePage",
|
||||
"renamePage",
|
||||
"movePage",
|
||||
"deletePage",
|
||||
"editPageText",
|
||||
"patchNode",
|
||||
"insertNode",
|
||||
"deleteNode",
|
||||
"updatePageJson",
|
||||
"tableInsertRow",
|
||||
"tableDeleteRow",
|
||||
"tableUpdateCell",
|
||||
"copyPageContent",
|
||||
"importPageMarkdown",
|
||||
"sharePage",
|
||||
"unsharePage",
|
||||
"restorePageVersion",
|
||||
"transformPage",
|
||||
"stashPage",
|
||||
// write (comment)
|
||||
"createComment",
|
||||
"resolveComment",
|
||||
];
|
||||
|
||||
test("DocmostClient implements every method the in-app DocmostClientLike mirror declares", () => {
|
||||
// The constructor is side-effect-free (no network/login on construction): it
|
||||
// only stores config and creates an axios instance, so it is safe to build a
|
||||
// throwaway instance here with a dummy token provider.
|
||||
const client = new DocmostClient({
|
||||
apiUrl: "http://127.0.0.1:1/api",
|
||||
getToken: async () => "test-token",
|
||||
});
|
||||
|
||||
const missing = HOST_CONTRACT_METHODS.filter(
|
||||
(name) => typeof client[name] !== "function",
|
||||
);
|
||||
|
||||
assert.deepEqual(
|
||||
missing,
|
||||
[],
|
||||
`DocmostClient is missing host-contract method(s): ${missing.join(", ")}. ` +
|
||||
`Update packages/mcp/src/client.ts and/or the server's DocmostClientLike ` +
|
||||
`interface (apps/server/src/core/ai-chat/tools/docmost-client.loader.ts) ` +
|
||||
`so the hand-mirrored method NAMES stay aligned (this guards names only, ` +
|
||||
`not signatures).`,
|
||||
);
|
||||
});
|
||||
|
||||
test("HOST_CONTRACT_METHODS has no duplicates", () => {
|
||||
assert.equal(
|
||||
new Set(HOST_CONTRACT_METHODS).size,
|
||||
HOST_CONTRACT_METHODS.length,
|
||||
);
|
||||
});
|
||||
|
||||
// Parse the method names declared in the server's `DocmostClientLike` interface
|
||||
// body. We read the .ts source as plain text (no TS compiler dep, and the file
|
||||
// lives in the CJS server tree across the ESM boundary): scan from the
|
||||
// `export interface DocmostClientLike {` line to its closing brace at column 0,
|
||||
// matching member-signature lines like ` methodName(`. Nested param-object
|
||||
// braces (`opts: { ... }`) are indented, so only the interface's own closing
|
||||
// `}` (column 0) ends the scan.
|
||||
function parseDocmostClientLikeMethods() {
|
||||
const here = dirname(fileURLToPath(import.meta.url));
|
||||
// packages/mcp/test/unit -> repo root is four levels up.
|
||||
const loaderPath = resolve(
|
||||
here,
|
||||
"../../../../apps/server/src/core/ai-chat/tools/docmost-client.loader.ts",
|
||||
);
|
||||
let source;
|
||||
try {
|
||||
source = readFileSync(loaderPath, "utf8");
|
||||
} catch (err) {
|
||||
if (err && err.code === "ENOENT") {
|
||||
throw new Error(
|
||||
`Expected monorepo layout; server tree at ${loaderPath} not found. ` +
|
||||
`This drift-guard reads the server's DocmostClientLike interface via a ` +
|
||||
`fixed relative path and must run from inside the monorepo checkout.`,
|
||||
);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
const lines = source.split(/\r?\n/);
|
||||
|
||||
const startIdx = lines.findIndex((l) =>
|
||||
/^export interface DocmostClientLike\s*\{/.test(l),
|
||||
);
|
||||
assert.notEqual(
|
||||
startIdx,
|
||||
-1,
|
||||
`Could not find "export interface DocmostClientLike {" in ${loaderPath}. ` +
|
||||
`If the interface was renamed/moved, update this drift-guard test.`,
|
||||
);
|
||||
|
||||
const methods = [];
|
||||
let closed = false;
|
||||
// Track whether we are inside a `/* ... */` block comment. Inner lines of a
|
||||
// block comment need NOT start with `*`, so a `name(` line inside one would be
|
||||
// falsely parsed as an interface method without this. (`//` line comments can
|
||||
// never match the method regex below since they start with `/`.)
|
||||
let inBlockComment = false;
|
||||
for (let i = startIdx + 1; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
if (inBlockComment) {
|
||||
// Stay in the block until we see its closing `*/`.
|
||||
if (line.includes("*/")) inBlockComment = false;
|
||||
continue;
|
||||
}
|
||||
// Enter a block comment only when it opens without closing on the same line;
|
||||
// a self-contained `/* ... */` on one line cannot precede a method name we
|
||||
// care about (such lines start with `/`, so the method regex won't match).
|
||||
if (line.includes("/*") && !line.includes("*/")) {
|
||||
inBlockComment = true;
|
||||
continue;
|
||||
}
|
||||
if (/^\}/.test(line)) {
|
||||
closed = true;
|
||||
break;
|
||||
}
|
||||
// Method-name match: a TS identifier (letters/digits/`_`/`$`, not starting
|
||||
// with a digit) optionally followed by a generic clause (`method<T>(`), then
|
||||
// the opening paren of the signature.
|
||||
const m = /^\s*([A-Za-z_$][A-Za-z0-9_$]*)\s*(?:<[^>]*>)?\(/.exec(line);
|
||||
if (m) methods.push(m[1]);
|
||||
}
|
||||
assert.ok(
|
||||
closed,
|
||||
`Did not find the closing brace of DocmostClientLike in ${loaderPath}.`,
|
||||
);
|
||||
assert.ok(
|
||||
methods.length > 0,
|
||||
`Parsed zero methods from DocmostClientLike in ${loaderPath} — the parser ` +
|
||||
`is likely out of date with the interface formatting.`,
|
||||
);
|
||||
return methods;
|
||||
}
|
||||
|
||||
// The point of the guard is to protect the DocmostClientLike mirror <-> client.ts
|
||||
// link, but HOST_CONTRACT_METHODS is itself a HAND-COPY of that interface kept in
|
||||
// sync manually. The list<->interface link must be tested too: a method consumed
|
||||
// by the adapter and added to DocmostClientLike but forgotten here (or removed
|
||||
// from the interface but left here) would otherwise escape both the server
|
||||
// typecheck (pkg emits no .d.ts) and the first test above (name not in the list).
|
||||
// Assert the two agree BOTH ways.
|
||||
test("HOST_CONTRACT_METHODS exactly mirrors the server's DocmostClientLike interface", () => {
|
||||
const interfaceMethods = parseDocmostClientLikeMethods();
|
||||
assert.deepEqual(
|
||||
[...HOST_CONTRACT_METHODS].sort(),
|
||||
[...interfaceMethods].sort(),
|
||||
`HOST_CONTRACT_METHODS has drifted from the DocmostClientLike interface in ` +
|
||||
`apps/server/src/core/ai-chat/tools/docmost-client.loader.ts. Add/remove ` +
|
||||
`method names in HOST_CONTRACT_METHODS so it lists EXACTLY the methods ` +
|
||||
`declared in that interface (both directions are checked).`,
|
||||
);
|
||||
});
|
||||
@@ -167,6 +167,38 @@ test("export emits comment anchors and they round-trip back to a comment mark",
|
||||
});
|
||||
});
|
||||
|
||||
test("export emits a spoiler span and it round-trips back to a spoiler mark", () => {
|
||||
// A small ProseMirror doc with a text run carrying a `spoiler` mark. The MCP
|
||||
// schema mirrors the editor-ext mark, so a spoiler must survive json -> md ->
|
||||
// json instead of being silently dropped as an unrecognized mark.
|
||||
const doc = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
content: [
|
||||
{ type: "text", text: "plot: " },
|
||||
{
|
||||
type: "text",
|
||||
text: "the butler did it",
|
||||
marks: [{ type: "spoiler" }],
|
||||
},
|
||||
{ type: "text", text: " end" },
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const body = convertProseMirrorToMarkdown(doc);
|
||||
assert.match(body, /<span data-spoiler="true">the butler did it<\/span>/);
|
||||
|
||||
return markdownToProseMirror(body).then((rebuilt) => {
|
||||
const spoilered = findTextWithMark(rebuilt, "spoiler");
|
||||
assert.ok(spoilered, "expected a text node with a spoiler mark");
|
||||
assert.equal(spoilered.text, "the butler did it");
|
||||
});
|
||||
});
|
||||
|
||||
test("drawio round-trips through export and import", () => {
|
||||
const doc = {
|
||||
type: "doc",
|
||||
|
||||
@@ -149,3 +149,37 @@ test("empty task item still emits its marker", () => {
|
||||
|
||||
assert.equal(convertProseMirrorToMarkdown(input), "- [ ]\n- [x]");
|
||||
});
|
||||
|
||||
// Image captions (issue #221). An image WITHOUT a caption stays the lossy-free
|
||||
// ``; WITH a caption it is emitted as a raw <img data-caption>
|
||||
// wrapped in a block <div> (symmetric to video) so the round-trip md -> html ->
|
||||
// json restores the caption via the image extension's parseHTML.
|
||||
test("image without a caption emits plain ", () => {
|
||||
const input = doc({
|
||||
type: "image",
|
||||
attrs: { src: "/files/a.png", alt: "cat" },
|
||||
});
|
||||
assert.equal(convertProseMirrorToMarkdown(input), "");
|
||||
});
|
||||
|
||||
test("image with a caption emits a raw <img data-caption> in a block div", () => {
|
||||
const input = doc({
|
||||
type: "image",
|
||||
attrs: { src: "/files/a.png", alt: "cat", caption: "A grey cat" },
|
||||
});
|
||||
assert.equal(
|
||||
convertProseMirrorToMarkdown(input),
|
||||
'<div><img src="/files/a.png" alt="cat" data-caption="A grey cat"></div>',
|
||||
);
|
||||
});
|
||||
|
||||
test("image caption escapes & and \" in the data-caption attribute", () => {
|
||||
const input = doc({
|
||||
type: "image",
|
||||
attrs: { src: "/files/a.png", caption: 'Tom & "Jerry"' },
|
||||
});
|
||||
assert.equal(
|
||||
convertProseMirrorToMarkdown(input),
|
||||
'<div><img src="/files/a.png" data-caption="Tom & "Jerry""></div>',
|
||||
);
|
||||
});
|
||||
|
||||
@@ -142,3 +142,31 @@ test("round-trip: pdf node survives markdown export with src + name + attachment
|
||||
assert.equal(found[0].attrs?.name, "x.pdf");
|
||||
assert.equal(found[0].attrs?.attachmentId, "a4");
|
||||
});
|
||||
|
||||
// The converter emits captioned images as a raw <img data-caption="...">; for
|
||||
// the caption to survive the PM -> markdown -> PM round-trip the docmost-schema
|
||||
// Image node must parse data-caption back into the `caption` attr. Without that
|
||||
// (stock @tiptap/extension-image), the caption is silently lost — these guard
|
||||
// the "lossless" claim.
|
||||
test("round-trip: image caption survives markdown export (data-caption restored)", async () => {
|
||||
const found = await roundtrip(
|
||||
{ type: "image", attrs: { src: "/api/files/cat.png", alt: "cat", caption: "A grey cat" } },
|
||||
"image",
|
||||
);
|
||||
assert.equal(found.length, 1, "image node should survive");
|
||||
assert.equal(found[0].attrs?.src, "/api/files/cat.png");
|
||||
assert.equal(found[0].attrs?.caption, "A grey cat", "caption must round-trip");
|
||||
});
|
||||
|
||||
test("round-trip: image caption with special chars survives markdown export", async () => {
|
||||
const found = await roundtrip(
|
||||
{ type: "image", attrs: { src: "/api/files/cat.png", caption: 'Tom & "Jerry"' } },
|
||||
"image",
|
||||
);
|
||||
assert.equal(found.length, 1, "image node should survive");
|
||||
assert.equal(
|
||||
found[0].attrs?.caption,
|
||||
'Tom & "Jerry"',
|
||||
"special-char caption must round-trip unescaped",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -82,6 +82,24 @@ test("round-trip: image inside a column survives as an image node (not literal m
|
||||
assert.ok(!JSON.stringify(out).includes("![pic]"), "image must not become literal markdown text");
|
||||
});
|
||||
|
||||
test("round-trip: captioned image inside a column preserves its caption (imageToHtml branch)", async () => {
|
||||
// A captioned image in a column is emitted via the imageToHtml helper (raw
|
||||
// HTML container), a different path from the top-level image case. Special
|
||||
// chars in the caption exercise attribute escaping on the way out and in.
|
||||
const caption = 'Tom & "Jerry"';
|
||||
const input = doc({
|
||||
type: "columns",
|
||||
content: [
|
||||
{ type: "column", content: [{ type: "image", attrs: { src: "/api/files/a/p.png", alt: "pic", caption } }] },
|
||||
{ type: "column", content: [para(text("right"))] },
|
||||
],
|
||||
});
|
||||
const out = await roundtrip(input);
|
||||
const imgs = findNodes(out, "image");
|
||||
assert.equal(imgs.length, 1, "captioned image inside a column must survive");
|
||||
assert.equal(imgs[0].attrs?.caption, caption, "caption (incl. special chars) must be preserved");
|
||||
});
|
||||
|
||||
test("round-trip: blockquote inside a column survives as a blockquote node", async () => {
|
||||
const input = doc({
|
||||
type: "columns",
|
||||
|
||||
Reference in New Issue
Block a user