diff --git a/packages/mcp/src/lib/collaboration.ts b/packages/mcp/src/lib/collaboration.ts index e5219e25..21d14eec 100644 --- a/packages/mcp/src/lib/collaboration.ts +++ b/packages/mcp/src/lib/collaboration.ts @@ -12,7 +12,7 @@ import { JSDOM } from "jsdom"; // canonicalization wrapper stay mcp-side. import { markdownToProseMirror, - normalizeForeignMarkdown, + normalizeAgentMarkdown, } from "@docmost/prosemirror-markdown"; import { docmostExtensions, docmostSchema } from "./docmost-schema.js"; import { withPageLock } from "./page-lock.js"; @@ -102,10 +102,14 @@ global.WebSocket = WebSocket; * `^[body]` footnotes carry their body at the reference point, so a comment can * no longer produce a reference-less footnote definition to be dropped. * - * #493: `normalizeForeignMarkdown` runs FIRST, so an agent's `updatePageMarkdown` - * body is normalized exactly like the server import path — GFM `[^id]` reference - * footnotes become canonical inline `^[body]`, and a leading YAML front-matter - * block is stripped — instead of leaking through as literal text / a bogus link. + * #493: `normalizeAgentMarkdown` runs FIRST, so an agent's `updatePageMarkdown` + * body gets the SAME GFM `[^id]` reference-footnote -> inline `^[body]` rewrite as + * the server import path (instead of the reference leaking as literal text / a + * bogus link). It DELIBERATELY does NOT strip a leading YAML front-matter block: + * a full-body agent rewrite that opens with a `---…---` is (almost) always a + * horizontalRule the serializer emitted, and stripping it would silently drop the + * page's leading content (#493 review). The front-matter strip stays on the + * server FILE-import boundary only (`normalizeForeignMarkdown`). */ export async function markdownToProseMirrorCanonical( markdownContent: string, @@ -115,7 +119,7 @@ export async function markdownToProseMirrorCanonical( // now-orphaned duplicate definitions. return canonicalizeFootnotes( normalizeAndMergeFootnotes( - await markdownToProseMirror(normalizeForeignMarkdown(markdownContent)), + await markdownToProseMirror(normalizeAgentMarkdown(markdownContent)), ), ); } diff --git a/packages/prosemirror-markdown/src/lib/foreign-markdown.ts b/packages/prosemirror-markdown/src/lib/foreign-markdown.ts index 43998966..a49bc4e3 100644 --- a/packages/prosemirror-markdown/src/lib/foreign-markdown.ts +++ b/packages/prosemirror-markdown/src/lib/foreign-markdown.ts @@ -254,11 +254,18 @@ function convertReferenceFootnotes(markdown: string): string { const YAML_FRONT_MATTER_RE = /^\uFEFF?---\n[\s\S]*?\n---\n?/; /** - * Normalize a foreign markdown string into Docmost's canonical markdown surface - * so the strict canonical parser accepts it losslessly: normalize line endings, - * strip a leading YAML front-matter block, then rewrite GFM reference footnotes - * into inline footnotes. Add further fixture-driven foreign-surface cases here as - * they are found. + * Normalize a foreign markdown string from a FILE IMPORT into Docmost's canonical + * markdown surface so the strict canonical parser accepts it losslessly: normalize + * line endings, strip a leading YAML front-matter block, then rewrite GFM reference + * footnotes into inline footnotes. Add further fixture-driven foreign-surface cases + * here as they are found. + * + * FRONT-MATTER STRIP IS IMPORT-ONLY (#493 review): use this ONLY at the server + * file-import boundary, where a `.md` file really can open with an Obsidian/Hugo + * YAML header. Do NOT use it on the canonical AGENT-WRITE path — see + * {@link normalizeAgentMarkdown} for why a full-body agent rewrite must NOT strip + * a leading `---…---` (it is normally a horizontalRule the serializer emitted, and + * stripping it would silently drop the page's leading content). */ export function normalizeForeignMarkdown(markdown: string): string { if (!markdown) return markdown; @@ -271,3 +278,26 @@ export function normalizeForeignMarkdown(markdown: string): string { const withoutFrontMatter = src.replace(YAML_FRONT_MATTER_RE, '').trimStart(); return convertReferenceFootnotes(withoutFrontMatter); } + +/** + * Canonical AGENT-WRITE normalization: normalize line endings and rewrite GFM + * `[^id]` reference footnotes to inline `^[body]` — but DELIBERATELY NOT strip a + * leading YAML front-matter block. + * + * WHY the split (#493 review): the reference-footnote rewrite is the drift the + * MCP page-write path (`updatePageMarkdown` -> `markdownToProseMirrorCanonical`) + * needed unified with the server import (an agent may paste GFM footnotes). The + * front-matter strip, however, is a FILE-import concern: on a full-body agent + * rewrite a leading `---…---` is (almost) always a `horizontalRule` the + * serializer emitted plus a later rule/heading — NOT a foreign YAML header — so + * `YAML_FRONT_MATTER_RE` would match it and SILENTLY DELETE the page's leading + * content (a page that starts with a horizontal rule and contains a second `---` + * lost everything up to it). Agent writes must never lose already-stored content, + * so this variant skips the strip. It IS a no-op on canonical serialized content + * (which never emits `[^id]:` reference-definition lines). + */ +export function normalizeAgentMarkdown(markdown: string): string { + if (!markdown) return markdown; + const src = markdown.replace(/\r\n/g, '\n'); + return convertReferenceFootnotes(src); +} diff --git a/packages/prosemirror-markdown/src/lib/index.ts b/packages/prosemirror-markdown/src/lib/index.ts index 2c5c2bf0..8bdfeb14 100644 --- a/packages/prosemirror-markdown/src/lib/index.ts +++ b/packages/prosemirror-markdown/src/lib/index.ts @@ -27,11 +27,17 @@ export { } from "./markdown-to-prosemirror.js"; // Foreign-markdown normalizer (#493): the input-liberal pre-pass that rewrites -// GFM `[^id]` reference footnotes to canonical inline `^[body]` and strips a -// leading YAML front-matter block, run at every canonical IMPORT boundary -// (server import + mcp `markdownToProseMirrorCanonical`) so foreign markdown is -// normalized identically everywhere instead of only in apps/server. -export { normalizeForeignMarkdown } from "./foreign-markdown.js"; +// GFM `[^id]` reference footnotes to canonical inline `^[body]`. Two variants: +// `normalizeForeignMarkdown` (server FILE-import boundary) ALSO strips a leading +// YAML front-matter block; `normalizeAgentMarkdown` (canonical AGENT-WRITE path, +// mcp `markdownToProseMirrorCanonical`) does NOT — a full-body agent rewrite must +// not lose a leading `---…---` horizontalRule to the front-matter strip (#493 +// review). The reference-footnote rewrite is shared so agent + import stay unified +// where it matters, without the content-losing strip on the write path. +export { + normalizeForeignMarkdown, + normalizeAgentMarkdown, +} from "./foreign-markdown.js"; // The Docmost tiptap schema mirror. Exposed so consumers (and the sync // engine's schema-validity regression tests) can build the exact ProseMirror diff --git a/packages/prosemirror-markdown/test/foreign-markdown.test.ts b/packages/prosemirror-markdown/test/foreign-markdown.test.ts index 5718b67e..135233fb 100644 --- a/packages/prosemirror-markdown/test/foreign-markdown.test.ts +++ b/packages/prosemirror-markdown/test/foreign-markdown.test.ts @@ -1,7 +1,10 @@ import { describe, it, expect } from 'vitest'; import { convertProseMirrorToMarkdown } from '../src/lib/markdown-converter.js'; import { markdownToProseMirror } from '../src/lib/markdown-to-prosemirror.js'; -import { normalizeForeignMarkdown } from '../src/lib/foreign-markdown.js'; +import { + normalizeForeignMarkdown, + normalizeAgentMarkdown, +} from '../src/lib/foreign-markdown.js'; /** * STEP 2 goldens for issue #345 (moved into the package with the normalizer in @@ -216,3 +219,53 @@ describe('foreign markdown import acceptance (normalizer + canonical parser)', ( ).toHaveLength(1); }); }); + +describe('normalizeAgentMarkdown vs normalizeForeignMarkdown — front-matter strip is IMPORT-only (#493 review)', () => { + // A page that OPENS with a horizontalRule and contains a later `---` serializes + // to a `---…---`-shaped body. On a full-body AGENT rewrite this must NOT be + // mistaken for YAML front-matter and stripped — that silently dropped the + // page's leading content. + const rulePage = '---\n\nIntro\n\nMore\n\n---\n\nRest'; + + it('normalizeAgentMarkdown does NOT strip a leading ---…--- (no content loss)', () => { + expect(normalizeAgentMarkdown(rulePage)).toBe(rulePage); + }); + + it('normalizeForeignMarkdown (file import) STILL strips a real leading YAML front-matter block', () => { + const withYaml = '---\ntitle: My Page\ntags: [a, b]\n---\n\nBody here.'; + const out = normalizeForeignMarkdown(withYaml); + expect(out).toBe('Body here.'); + // And the horizontalRule-shaped body IS stripped on the import path (its + // documented file-import behavior) — the two variants differ ONLY here. + expect(normalizeForeignMarkdown(rulePage)).not.toContain('Intro'); + }); + + it('agent-write round-trip keeps a horizontalRule-led doc with a second rule intact', async () => { + // Simulate the serializer output for [horizontalRule, para, para, horizontalRule, para]. + const doc = { + type: 'doc', + content: [ + { type: 'horizontalRule' }, + { type: 'paragraph', content: [{ type: 'text', text: 'Intro' }] }, + { type: 'paragraph', content: [{ type: 'text', text: 'More' }] }, + { type: 'horizontalRule' }, + { type: 'paragraph', content: [{ type: 'text', text: 'Rest' }] }, + ], + }; + const body = convertProseMirrorToMarkdown(doc); + // The agent-write normalization must NOT eat the head; re-import keeps every + // paragraph's text. + const back = await markdownToProseMirror(normalizeAgentMarkdown(body)); + const texts = JSON.stringify(back); + for (const t of ['Intro', 'More', 'Rest']) expect(texts).toContain(t); + // Both horizontal rules survive. + expect(back.content.filter((n: any) => n.type === 'horizontalRule')).toHaveLength(2); + }); + + it('agent-write STILL rewrites GFM reference footnotes (the shared drift-fix)', () => { + const gfm = 'See[^1].\n\n[^1]: the note.'; + const out = normalizeAgentMarkdown(gfm); + expect(out).toContain('^[the note.]'); + expect(out).not.toMatch(/\[\^1\]:/); + }); +});