diff --git a/AGENTS.md b/AGENTS.md index 064c3ccd..642b927f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -334,7 +334,7 @@ pnpm workspace (`pnpm@10.4.0`) orchestrated by **Nx**. Four workspace packages: | `apps/client` | `client` | React 18 + Vite + Mantine 8 + TanStack Query + Jotai | SPA frontend | | `packages/editor-ext` | `@docmost/editor-ext` | Tiptap/ProseMirror | Shared Tiptap node/mark extensions, imported by both the client and the server | | `packages/mcp` | `@docmost/mcp` | MCP SDK, Tiptap, Yjs | Standalone MCP server, also bundled into the server at `/mcp`. Consumes the shared converter/schema from `@docmost/prosemirror-markdown` (#293) — it no longer carries its own vendored converter/schema copy | -| `packages/prosemirror-markdown` | `@docmost/prosemirror-markdown` | Tiptap, marked, jsdom | The single, canonical ProseMirror↔Markdown converter + Docmost schema mirror (#293). Consumed by `mcp`, `git-sync`, AND `apps/server` (server-side markdown import/export, #345); there is exactly ONE copy of the converter now | +| `packages/prosemirror-markdown` | `@docmost/prosemirror-markdown` | Tiptap, marked; jsdom (Node only) | The single, canonical ProseMirror↔Markdown converter + Docmost schema mirror (#293). Consumed by `mcp`, `git-sync`, `apps/server` (server-side markdown import/export, #345), AND `apps/client` (markdown paste/copy + AI-chat render, via the `browser` entry — native `DOMParser`, no jsdom in the client bundle, #347); there is exactly ONE copy of the converter now | `build` targets are Nx-cached and dependency-ordered (`dependsOn: ["^build"]`), so `editor-ext` builds before the apps. `nx.json` sets `affected.defaultBase: main`. @@ -460,7 +460,7 @@ The API server is a Fastify app with a global `/api` prefix (`main.ts` excludes ### Client structure Vite SPA. Code is organized by feature under `apps/client/src/features/*` (mirrors the server domains: `page`, `space`, `comment`, `ai-chat`, `editor`, …). Conventions: - **TanStack Query** for server state (one `queries/` file per feature), **Jotai** atoms for local/shared UI state, **Mantine 8** + CSS modules (`*.module.css`) + `postcss-preset-mantine` for UI. -- The editor is Tiptap; shared node/mark extensions live in `packages/editor-ext` and are imported by **both the client and the server** (collaboration, schema, `canonicalizeFootnotes`) — editor schema changes often need to be made in `editor-ext`, not just the client. Server-side markdown import/export no longer lives in `editor-ext`: it goes through the canonical converter (#345, see below). The ProseMirror↔Markdown converter and its Docmost schema mirror now live in a SINGLE package, `@docmost/prosemirror-markdown` (#293), consumed by `mcp`, `git-sync`, and `apps/server` (#345) — do NOT reintroduce a per-package copy. `editor-ext` is the upstream source of the Tiptap schema; the package's `docmost-schema.ts` mirrors it and a serializer-contract test (`packages/prosemirror-markdown/test/serializer-contract.test.ts`) guards the boundary (every schema node must have a converter case), so a drift surfaces as a failing test rather than silent divergence. For the converter's property-testing and counterexample→fixture process (P1–P4 invariants, the `PROPERTY_SEED`/`PROPERTY_NUM_RUNS` knobs, and the nightly fuzz workflow), see `packages/prosemirror-markdown/README.md`. +- The editor is Tiptap; shared node/mark extensions live in `packages/editor-ext` and are imported by **both the client and the server** (collaboration, schema, `canonicalizeFootnotes`) — editor schema changes often need to be made in `editor-ext`, not just the client. Server-side markdown import/export no longer lives in `editor-ext`: it goes through the canonical converter (#345, see below). The ProseMirror↔Markdown converter and its Docmost schema mirror now live in a SINGLE package, `@docmost/prosemirror-markdown` (#293), consumed by `mcp`, `git-sync`, `apps/server` (#345), and `apps/client` (#347) — do NOT reintroduce a per-package copy. The client uses the package's `browser` entry (`@docmost/prosemirror-markdown/browser`): markdown paste (`markdown-clipboard.ts`), copy-as-markdown, and AI-chat rendering now all go through the canonical converter, so the hand-written `marked`/`turndown` markdown layer that used to live in `editor-ext` was deleted (#347). The browser entry runs the HTML→DOM stage on the native `DOMParser`, so jsdom stays out of the client bundle. `editor-ext` is the upstream source of the Tiptap schema; the package's `docmost-schema.ts` mirrors it and a serializer-contract test (`packages/prosemirror-markdown/test/serializer-contract.test.ts`) guards the boundary (every schema node must have a converter case), so a drift surfaces as a failing test rather than silent divergence. For the converter's property-testing and counterexample→fixture process (P1–P4 invariants, the `PROPERTY_SEED`/`PROPERTY_NUM_RUNS` knobs, and the nightly fuzz workflow), see `packages/prosemirror-markdown/README.md`. - API access goes through `apps/client/src/lib/api-client.ts` (axios). The `@` alias maps to `apps/client/src`. - Runtime config is injected at build time by `vite.config.ts` via `define` (`APP_URL`, `COLLAB_URL`, `APP_VERSION`, …) — these come from the root `.env`, not from `import.meta.env`. diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b44878a..795c7502 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -270,6 +270,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Client markdown paste/copy and AI-chat rendering now go through the canonical + converter.** Pasting markdown into the editor, "Copy as markdown", the AI title + generator, and the AI-chat markdown renderer all now use + `@docmost/prosemirror-markdown` (via its new `browser` entry — native + `DOMParser`, no jsdom in the client bundle) instead of the hand-written + `marked`/`turndown` markdown layer in `editor-ext`, which was **deleted**. As a + result, pasting canonical markdown (`^[…]` footnotes, ``, + `> [!type]` callouts, `$…$` math, `==…==` highlight, standalone `` + comments) now produces the SAME nodes the server import produces for the same + text. Chat/reasoning markdown now renders through the editor schema (list items + are wrapped in `

`; CSS keeps them tight). (#347) + - **Enabling a public share no longer auto-shares the whole sub-tree.** Turning a page "Shared to web" now defaults to the page alone; descendant pages become public only when you explicitly turn on the dedicated "Include sub-pages" diff --git a/apps/client/src/features/ai-chat/components/ai-chat.module.css b/apps/client/src/features/ai-chat/components/ai-chat.module.css index 94affdb9..fa9e4f10 100644 --- a/apps/client/src/features/ai-chat/components/ai-chat.module.css +++ b/apps/client/src/features/ai-chat/components/ai-chat.module.css @@ -181,6 +181,14 @@ margin: 0 0 4px; } +/* Same as `.markdown li p` above: the canonical converter wraps every list + item's content in a

, so without this each reasoning-panel list item would + pick up `.reasoningText p`'s 4px bottom margin and render too loose. Drop it + so Reasoning-panel lists stay tight, mirroring the pre-#347 marked output. */ +.reasoningText li p { + margin: 0; +} + .inputWrapper { flex: 0 0 auto; padding-top: var(--mantine-spacing-xs); diff --git a/apps/client/src/features/editor/extensions/markdown-clipboard.paste.test.ts b/apps/client/src/features/editor/extensions/markdown-clipboard.paste.test.ts index 15c1d0d2..be897ade 100644 --- a/apps/client/src/features/editor/extensions/markdown-clipboard.paste.test.ts +++ b/apps/client/src/features/editor/extensions/markdown-clipboard.paste.test.ts @@ -110,3 +110,69 @@ describe("MarkdownClipboard handlePaste (async md -> PM)", () => { editor.destroy(); }); }); + +// The async seam captures the target range synchronously, then replaces on the +// next microtask. If the document changed under it between capture and resolve +// (impossible in prod — same microtask — but pinned here), BOTH the success +// (replaceRange) and the fail-open (insertText) branches must fall back to the +// LIVE selection rather than a stale absolute range, so neither clobbers content +// nor throws a RangeError. We force the mid-flight change by dispatching a +// doc-mutating transaction AFTER the synchronous claim but BEFORE flushing the +// microtask that runs the `.then`/`.catch`. +describe("MarkdownClipboard handlePaste — doc-changed-mid-flight guard", () => { + // Insert marker text at the doc start via a raw transaction (synchronous), + // changing `view.state.doc` so the captured range goes stale. + function mutateDoc(editor: Editor, marker: string) { + editor.view.dispatch(editor.view.state.tr.insertText(marker, 1)); + } + + it("success branch: a mid-flight doc change routes the paste to the live selection (no clobber, no throw)", async () => { + const editor = makeEditor(); + const claimed = paste(editor, "hello **bold**"); + expect(claimed).toBe(true); + // Doc changes before the async replace runs: the captured from/to are stale. + mutateDoc(editor, "MARKER"); + await flush(); + + const text = editor.getText(); + // The pre-existing marker survived (a stale-range replaceRange would have + // clobbered it) AND the pasted content landed. + expect(text).toContain("MARKER"); + expect(text).toContain("bold"); + expect(text).not.toContain("**"); + editor.destroy(); + }); + + it("fail-open branch: a mid-flight doc change + conversion failure re-inserts raw text at the live selection (no RangeError)", async () => { + const editor = makeEditor(); + // `# heading` -> a heading node the minimal schema lacks -> PMNode.fromJSON + // throws -> the fail-open catch runs, now with a changed doc. + paste(editor, "# raw heading"); + mutateDoc(editor, "KEEP"); + await flush(); + + const text = editor.getText(); + // No RangeError/unhandled rejection (the test would fail on a throw), the + // marker survived, and the raw text was preserved. + expect(text).toContain("KEEP"); + expect(text).toContain("raw heading"); + editor.destroy(); + }); + + it("two pastes in flight: neither payload is lost (no data loss)", async () => { + // Prod-unreachable (two paste events are separate macrotasks, and each + // conversion resolves on a microtask before the next), but pinned here: when + // both resolve back-to-back, the second sees the changed doc and inserts at + // the live selection the first left — so the two payloads may INTERLEAVE, but + // neither is dropped. We assert no data loss, not contiguity. + const editor = makeEditor(); + paste(editor, "alphaword"); + paste(editor, "betaword"); + await flush(); + const text = editor.getText(); + // Neither payload fully dropped (interleaving may split one of them). + expect(text).toContain("alpha"); + expect(text).toContain("beta"); + editor.destroy(); + }); +}); diff --git a/apps/client/src/features/editor/extensions/markdown-clipboard.ts b/apps/client/src/features/editor/extensions/markdown-clipboard.ts index f9b8b62c..92ef71de 100644 --- a/apps/client/src/features/editor/extensions/markdown-clipboard.ts +++ b/apps/client/src/features/editor/extensions/markdown-clipboard.ts @@ -103,12 +103,14 @@ export const MarkdownClipboard = Extension.create({ } const schema = this.editor.schema; - // Capture the target range NOW. markdownToProseMirror is async (its - // marked pipeline is declared async), so the actual replace happens - // on the next microtask. No user input can interleave a microtask, so - // the state is unchanged when we dispatch — but we still re-read the - // live state and map the captured range through any doc steps for - // safety before replacing. + // Capture the target range NOW. markdownToProseMirror RETURNS A + // PROMISE (kept async only for the Node consumers' contract; the + // conversion pipeline itself is synchronous), so the actual replace + // happens on the next microtask. No user input can interleave a + // microtask, so the state is unchanged when we dispatch — but we + // still re-read the live state before replacing and, if the doc did + // change under us, fall back to the live selection rather than the + // captured (now-stale) range. const from = view.state.selection.from; const to = view.state.selection.to; const startDoc = view.state.doc; @@ -154,9 +156,10 @@ export const MarkdownClipboard = Extension.create({ schema, ); - // Map the captured range through any doc changes since capture - // (normally none — same microtask) so the replace targets the - // right span even if the document moved. + // Target the captured range (normally still valid — same + // microtask). If the doc changed under us since capture, the + // captured absolute from/to are stale, so fall back to the live + // selection rather than StepMap-mapping the old range. const tr = view.state.tr; let mappedFrom = from; let mappedTo = to; @@ -177,11 +180,20 @@ export const MarkdownClipboard = Extension.create({ tr.setMeta("paste", true); view.dispatch(tr); }) - .catch(() => { + .catch((err) => { // Fail-open: a conversion error must not swallow the paste // silently in a way that loses the text. We already claimed the // event (returned true), so re-insert the raw text as a plain // paragraph so the user never loses their clipboard content. + // Log it: this catch covers BOTH the converter and the success + // `.then` body (e.g. PMNode.fromJSON throwing on a schema drift + // between the canonical package and the live editor schema), so a + // silent degrade to raw text would otherwise be an invisible, + // non-reproducible regression ("my table pasted as text"). + console.error( + "markdown paste conversion failed, inserting raw text", + err, + ); if (view.isDestroyed) return; const tr = view.state.tr; // Same guard the success path uses: if the doc changed under us diff --git a/apps/server/src/integrations/import/utils/foreign-markdown.ts b/apps/server/src/integrations/import/utils/foreign-markdown.ts index 2173bfdc..dd7b012a 100644 --- a/apps/server/src/integrations/import/utils/foreign-markdown.ts +++ b/apps/server/src/integrations/import/utils/foreign-markdown.ts @@ -238,8 +238,9 @@ function convertReferenceFootnotes(markdown: string): string { * * LINE-ANCHORED (the same shape the canonical parser uses in * prosemirror-markdown/page-file.ts): the block opens only on `---\n` at the - * very start and closes only on a `\n---` line. The retired `markdownToHtml` - * strip closed on the FIRST `---` ANYWHERE (an unanchored close), so a value + * very start and closes only on a `\n---` line. The retired editor-ext + * `markdownToHtml` front-matter strip (removed in #347) closed on the FIRST + * `---` ANYWHERE (an unanchored close), so a value * containing a triple-dash (e.g. `title: Q1 --- Q2`) truncated the front-matter * and leaked the rest into the body. An optional leading BOM is tolerated. */ diff --git a/packages/editor-ext/src/lib/footnote/footnote-canonicalize.ts b/packages/editor-ext/src/lib/footnote/footnote-canonicalize.ts index f7a05f94..b38fdb5d 100644 --- a/packages/editor-ext/src/lib/footnote/footnote-canonicalize.ts +++ b/packages/editor-ext/src/lib/footnote/footnote-canonicalize.ts @@ -14,7 +14,8 @@ import { * ProseMirror JSON directly (never running the editor's plugins), so the * canonical footnote topology was never enforced on those writes. The consumers * of this editor-ext copy are: the server markdown/HTML import - * (`markdownToHtml -> htmlToJson` in import.service / file-import-task.service), + * (`markdownToProseMirror` from @docmost/prosemirror-markdown in import.service / + * file-import-task.service), * `PageService` create/update (`parseProsemirrorContent` for the JSON/markdown/ * HTML REST write paths), and the client markdown PASTE path * (`markdown-clipboard.ts`). (The MCP package mirrors this canonicalizer in diff --git a/packages/editor-ext/src/lib/footnote/footnote-sync.ts b/packages/editor-ext/src/lib/footnote/footnote-sync.ts index d0891e1a..891d8f9c 100644 --- a/packages/editor-ext/src/lib/footnote/footnote-sync.ts +++ b/packages/editor-ext/src/lib/footnote/footnote-sync.ts @@ -103,8 +103,9 @@ interface CollisionPlan { * `X__2`, `X__3`, collision-bumped) so it survives as a distinct footnote — which, * having no matching reference, then falls under the normal orphan policy. It is * only ever dropped for lacking a reference, never for colliding. The IMPORT - * paths (footnote.marked.ts / MCP extractFootnotes) instead apply first-wins + - * drop + warn for duplicate definitions; that divergence is intentional — import + * paths (@docmost/prosemirror-markdown / MCP extractFootnotes) instead apply + * first-wins + drop + warn for duplicate definitions; that divergence is + * intentional — import * is an agent-authored artifact we sanitize, the editor is live user data we must * not lose. * diff --git a/packages/editor-ext/src/lib/footnote/footnote-util.derive-id.test.ts b/packages/editor-ext/src/lib/footnote/footnote-util.derive-id.test.ts index 96d448ae..4cbb5605 100644 --- a/packages/editor-ext/src/lib/footnote/footnote-util.derive-id.test.ts +++ b/packages/editor-ext/src/lib/footnote/footnote-util.derive-id.test.ts @@ -6,8 +6,9 @@ import { deriveFootnoteId } from "./footnote-util"; * * `deriveFootnoteId` lives ONLY in editor-ext now — it is used by * `resolveCollisions` (re-id of a duplicate definition) and `footnotePastePlugin` - * (re-id of a pasted colliding definition). The MCP/marked import paths no longer - * derive ids (duplicate definitions there are first-wins-dropped, #166), so there + * (re-id of a pasted colliding definition). The MCP / @docmost/prosemirror-markdown + * import paths no longer derive ids (duplicate definitions there are + * first-wins-dropped, #166), so there * is no cross-package copy and no parity test to keep in sync. This table pins the * deterministic scheme so a future change to it is a conscious one. */ diff --git a/packages/editor-ext/src/lib/footnote/footnote-util.ts b/packages/editor-ext/src/lib/footnote/footnote-util.ts index d27c9685..0136c3d0 100644 --- a/packages/editor-ext/src/lib/footnote/footnote-util.ts +++ b/packages/editor-ext/src/lib/footnote/footnote-util.ts @@ -63,8 +63,9 @@ export function generateFootnoteId(): string { * its own seen-set before requesting the next derived id. * * Used only inside editor-ext now (resolveCollisions for a re-id'd duplicate - * DEFINITION, and footnotePastePlugin). The MCP/marked import paths no longer - * derive ids — duplicate definitions there are first-wins-dropped (#166) — so + * DEFINITION, and footnotePastePlugin). The MCP / @docmost/prosemirror-markdown + * import paths no longer derive ids — duplicate definitions there are + * first-wins-dropped (#166) — so * there is no cross-package copy to keep in sync. The golden table in * footnote-util.derive-id.test.ts pins the scheme. */ diff --git a/packages/prosemirror-markdown/README.md b/packages/prosemirror-markdown/README.md index 0c60d7d6..0d28b288 100644 --- a/packages/prosemirror-markdown/README.md +++ b/packages/prosemirror-markdown/README.md @@ -1,12 +1,31 @@ # @docmost/prosemirror-markdown The single, canonical **ProseMirror ↔ Markdown converter** plus the Docmost -schema mirror (#293/#345). Headless and framework-free: no React, no browser -runtime. There is exactly ONE copy of this converter in the repo, consumed by: +schema mirror (#293/#345/#347). Headless and framework-free: no React. There is +exactly ONE copy of this converter in the repo, consumed by: - `packages/mcp` (the MCP server), - `packages/git-sync` (two-way Git sync), -- `apps/server` (server-side markdown import/export, #345). +- `apps/server` (server-side markdown import/export, #345), +- `apps/client` (markdown paste/copy + AI-chat render, #347). + +### Node vs browser entry + +The HTML→DOM stage of markdown import runs on `jsdom` in Node and the native +`DOMParser` in the browser, injected per environment so **jsdom never enters a +client bundle**: + +- default entry (`@docmost/prosemirror-markdown`) — Node: registers jsdom + + `@tiptap/html`'s happy-dom `server` `generateJSON`. Used by mcp / git-sync / + apps/server. +- `browser` entry (`@docmost/prosemirror-markdown/browser`, via the `"browser"` + exports condition) — registers the native `DOMParser` + `@tiptap/html`'s + browser `generateJSON`. Used by `apps/client`; carries no jsdom/happy-dom. + +Both entries expose the identical converter surface; only the injected +DOM/`generateJSON` implementations differ (`src/lib/dom-parser.ts`). A +`markdownToProseMirrorSync` variant exists for callers that cannot await (the +client's synchronous chat renderer). `src/lib/docmost-schema.ts` **mirrors** the upstream Tiptap schema that lives in `packages/editor-ext`. The mirror is not free-floating: `serializer-contract.test.ts` diff --git a/packages/prosemirror-markdown/src/lib/docmost-schema.ts b/packages/prosemirror-markdown/src/lib/docmost-schema.ts index 9b7dd006..882902ff 100644 --- a/packages/prosemirror-markdown/src/lib/docmost-schema.ts +++ b/packages/prosemirror-markdown/src/lib/docmost-schema.ts @@ -63,10 +63,9 @@ function getStyleProperty(element: HTMLElement, propertyName: string): string | * The editor SCHEMA genuinely only supports these six banner types — there is no * `tip`/`caution`/`important`/`question` callout node. So those are NOT first- * class types we can round-trip literally; they are INPUT ALIASES (GitHub/Obsidian - * alert syntax). The editor's own paste/import path maps them onto the supported - * set (see `GITHUB_ALERT_TYPE_MAP` in - * `@docmost/editor-ext` markdown/utils/github-callout.marked.ts: - * tip -> success, caution -> danger, important -> info). We mirror that aliasing + * alert syntax). This package's own `> [!type]` import path maps them onto the + * supported set (see `CALLOUT_TYPE_ALIASES` below: tip -> success, caution -> + * danger, important -> info). We apply that aliasing * here so an ingested `> [!tip]` / `> [!caution]` lands on the closest real banner * (success / danger) instead of flatly collapsing to `info` — matching exactly how * the editor itself would interpret the same alias. A schema type always maps to @@ -75,11 +74,11 @@ function getStyleProperty(element: HTMLElement, propertyName: string): string | */ const CALLOUT_TYPES = ["default", "info", "note", "success", "warning", "danger"]; /** - * NON-schema callout aliases -> their closest supported banner. Mirrors the - * editor's `GITHUB_ALERT_TYPE_MAP` for the names that are NOT already schema - * types (a schema type is preserved as-is and never consulted here). Keeping - * these in lockstep means git-sync ingest and an editor paste interpret the same - * `> [!alias]` identically. + * NON-schema callout aliases -> their closest supported banner, for the names + * that are NOT already schema types (a schema type is preserved as-is and never + * consulted here). This is the single canonical alias map now that the editor's + * old marked layer is gone; git-sync ingest and an editor paste both go through + * this package, so they interpret the same `> [!alias]` identically. */ const CALLOUT_TYPE_ALIASES: Record = { tip: "success",