From 7ea030cc9e473a96f23a95043fab8e012fa8986e Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 00:36:34 +0300 Subject: [PATCH] =?UTF-8?q?fix(prosemirror-markdown):=20=D0=BF=D0=BE=D0=BC?= =?UTF-8?q?=D0=B5=D1=87=D0=B0=D1=82=D1=8C=20=D0=B2=D0=BD=D1=83=D1=82=D1=80?= =?UTF-8?q?=D0=B5=D0=BD=D0=BD=D0=B8=D0=B5=20=D1=81=D1=81=D1=8B=D0=BB=D0=BA?= =?UTF-8?q?=D0=B8=20=D0=BD=D0=B0=20=D1=81=D1=82=D1=80=D0=B0=D0=BD=D0=B8?= =?UTF-8?q?=D1=86=D1=8B=20=D0=BA=D0=B0=D0=BA=20internal=20=D0=BF=D1=80?= =?UTF-8?q?=D0=B8=20=D0=B8=D0=BC=D0=BF=D0=BE=D1=80=D1=82=D0=B5=20markdown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit При импорте markdown ссылка вида `[t](/s//p/)` сохранялась как внешняя (link-mark получал `internal:null`, `target:"_blank"`, `rel:"noopener noreferrer nofollow"`): открывалась новой вкладкой, без hover-превью и без участия в backlinks. Единственный путь к нативной внутренней ссылке был ручной JSON-патч. Чиню в общем конвертере пакета, через который проходят ВСЕ markdown-пути (MCP updatePageMarkdown/importPageMarkdown, patch/insertNode, тела комментариев, серверный REST create/update, single/zip-импорт, ai-chat, git-sync pull, вставка markdown в редактор) — все они получают исправление разом. - Новый модуль `internal-links.ts`: чистый `isInternalPagePath(href)` — якорный `^/s//p//?$`, СТРОГОЕ подмножество серверного `INTERNAL_LINK_REGEX` (apps/server/.../export/utils.ts), поэтому всё помеченное гарантированно бэклинкуется и переписывается при экспорте. Fail-toward-external: любая неоднозначность (scheme/host, `#`, `?`, бесспейсовый `/p/`, лишний сегмент, не-строка) остаётся внешней. - `markInternalLinks(doc)`: пост-обход готового ProseMirror-документа, помечает КАЖДУЮ text-ноду, покрытую внутренней ссылкой (включая случай вложенных bold/italic внутри ссылки) → `{internal:true, target:null, rel:null}`. Чистая и идемпотентная. - Разводка в `markdownToProseMirrorSync` после stripEmptyParagraphs. - §11 (обязательная сопутствующая правка): `internal:false` добавлен в `KNOWN_DEFAULTS.link` канонизатора — редакторные внешние ссылки хранят `internal:false`, теперь он канонизируется как absent/null/external, а load-bearing `internal:true` (не дефолт) переживает канонизацию. Тесты: accept/reject-пиннинг матчера (edge: trailing slash, `#`, `?`, `/p/x` без спейса, `https://h/p/x`, `/api/...`, uppercase, empty-space), subset-инвариант против серверного регэкспа, мультинодовая ссылка со вложенными марками, идемпотентность, полный конвертер (internal помечен / external нетронут / бэклинк-извлекаемость), комментарий-тело через общий путь, canonicalize §11. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/lib/canonicalize.ts | 8 + .../prosemirror-markdown/src/lib/index.ts | 5 + .../src/lib/internal-links.ts | 112 +++++++ .../src/lib/markdown-to-prosemirror.ts | 8 +- .../test/internal-links.test.ts | 316 ++++++++++++++++++ 5 files changed, 448 insertions(+), 1 deletion(-) create mode 100644 packages/prosemirror-markdown/src/lib/internal-links.ts create mode 100644 packages/prosemirror-markdown/test/internal-links.test.ts diff --git a/packages/prosemirror-markdown/src/lib/canonicalize.ts b/packages/prosemirror-markdown/src/lib/canonicalize.ts index 25fbc921..b1f7593e 100644 --- a/packages/prosemirror-markdown/src/lib/canonicalize.ts +++ b/packages/prosemirror-markdown/src/lib/canonicalize.ts @@ -56,6 +56,14 @@ const KNOWN_DEFAULTS: Record> = { link: { target: "_blank", rel: "noopener noreferrer nofollow", + // Editor-authored EXTERNAL links store `internal: false` (editor-ext link + // default `packages/editor-ext/src/lib/link.ts`), while an imported external + // link leaves `internal` absent/null. Both mean "external", so `internal: + // false` must normalize away exactly like `null`/absent — otherwise a stored + // `internal:false` link diverges from its re-import under + // `docsCanonicallyEqual` (false !== null). The internal marker `internal:true` + // is NON-default, so it is KEPT and survives canonicalization (#522 §11). + internal: false, }, comment: { resolved: false, diff --git a/packages/prosemirror-markdown/src/lib/index.ts b/packages/prosemirror-markdown/src/lib/index.ts index cbf8f462..83da46c4 100644 --- a/packages/prosemirror-markdown/src/lib/index.ts +++ b/packages/prosemirror-markdown/src/lib/index.ts @@ -23,6 +23,11 @@ export { markdownToProseMirrorSync, } from "./markdown-to-prosemirror.js"; +// Pure primitive: detect the unambiguously-internal wiki-page link path +// (`/s//p/`) and promote such link marks to their native internal +// form during import (#522). A strict subset of the server's INTERNAL_LINK_REGEX. +export { isInternalPagePath, markInternalLinks } from "./internal-links.js"; + // The Docmost tiptap schema mirror. Exposed so consumers (and the sync // engine's schema-validity regression tests) can build the exact ProseMirror // schema the converter targets. diff --git a/packages/prosemirror-markdown/src/lib/internal-links.ts b/packages/prosemirror-markdown/src/lib/internal-links.ts new file mode 100644 index 00000000..77b39994 --- /dev/null +++ b/packages/prosemirror-markdown/src/lib/internal-links.ts @@ -0,0 +1,112 @@ +/** + * Detect and mark unambiguously-internal wiki-page links during markdown import. + * + * WHY THIS EXISTS + * --------------- + * The markdown converter (`markdownToProseMirrorSync`) materializes every link + * with StarterKit's external defaults — `internal: null`, `target: "_blank"`, + * `rel: "noopener noreferrer nofollow"`. A markdown link that points at an + * internal wiki page written in its host-less, root-relative form + * (`[text](/s//p/)`) is therefore stored as EXTERNAL: it opens in + * a new tab, gets no hover-preview, and is invisible to the backlink graph + * (`extractInternalLinkSlugIds` only counts links whose mark carries + * `internal: true`). This module supplies the pure primitive that a post-walk in + * the converter uses to promote such links to their native internal form. + * + * WHERE THE CANON LIVES (a strict subset, on purpose) + * --------------------------------------------------- + * The authoritative definition of "what is an internal link path" is the + * server's `INTERNAL_LINK_REGEX` + * (`apps/server/src/integrations/export/utils.ts`): + * /^(https?:\/\/)?([^\/]+)?(\/s\/([^\/]+)\/)?p\/([a-zA-Z0-9-]+)\/?$/ + * This package is a lower layer than the server app and cannot import it, so the + * matcher below is a DOCUMENTED, STRICT SUBSET of that regex: it accepts only the + * space-qualified, root-relative page path `/s//p/` (with optional + * trailing slash). Because it is a subset, every link we mark internal here is + * guaranteed to also satisfy the server regex, hence guaranteed backlink-able and + * export-rewritable. A unit test pins the exact accept/reject set as the guard + * against drift. + * + * FAIL-TOWARD-EXTERNAL + * -------------------- + * We mark a link internal ONLY on an unambiguous anchored match. Any ambiguity + * (a scheme/host, `#anchor`, `?query`, a space-less `/p/`, a relative + * `p/`, a non-string href) leaves the link external. A false-external is a + * soft degradation (a new tab); a false-internal would produce broken SPA + * navigation, so "external" is the conservative default. + * + * Precedent for the target internal shape: the file importer already promotes + * internal anchors (`apps/server/src/integrations/import/utils/import-formatter.ts` + * `$a.attr('data-internal','true')`) and editor-ext reads it + * (`packages/editor-ext/src/lib/link.ts`). + */ + +/** + * Matches ONLY the space-qualified, root-relative internal page path + * `/s//p/` (with optional trailing slash). A STRICT SUBSET of the + * server's `INTERNAL_LINK_REGEX`: + * - `` = `[^/]+` (any non-slash segment, as in the server's group 4) + * - `` = `[a-zA-Z0-9-]+` (as in the server's group 5 / `extractPageSlugId`) + * Anchored on both ends so a scheme/host, a trailing `#anchor`/`?query`, or any + * extra path segment fails to match. + */ +const INTERNAL_PAGE_PATH = /^\/s\/[^/]+\/p\/[a-zA-Z0-9-]+\/?$/; + +/** + * True iff `href` is the unambiguous, space-qualified, root-relative internal + * page path. Pure and side-effect-free. Non-string input returns false. + */ +export function isInternalPagePath(href: unknown): boolean { + return typeof href === "string" && INTERNAL_PAGE_PATH.test(href); +} + +/** + * The native internal-link attribute overrides applied to a link mark whose href + * is an internal page path. Mirrors the manual JSON-patch form + * (`{internal:true, target:null, rel:null}`) and the editor-ext/file-importer + * precedent: internal links carry no `target`/`rel` (same-tab SPA navigation). + */ +const INTERNAL_LINK_ATTRS = { internal: true, target: null, rel: null } as const; + +/** + * In-place post-walk of a finished ProseMirror doc that promotes every + * unambiguously-internal link mark to its native internal form. + * + * A link that spans several text nodes (e.g. `[**bold** word](/s/x/p/abc)`) + * stores an equivalent link mark on EACH covered text node — and a covered node + * may carry nested marks (bold/italic) alongside the link. We therefore walk the + * entire tree and rewrite EVERY `link` mark whose href passes + * `isInternalPagePath`, so no covered segment is left external. + * + * Pure of external effects and IDEMPOTENT: re-running on an already-marked doc + * leaves it unchanged (an internal-path href always maps to the same attrs). + * External links are never touched — their `target:_blank`/`rel:noopener…` stay. + * + * The doc is mutated in place (the converter owns the freshly-built doc and + * returns it directly); the same node reference is returned for convenience. + */ +export function markInternalLinks(node: T): T { + walk(node); + return node; +} + +function walk(node: any): void { + if (!node || typeof node !== "object") return; + if (Array.isArray(node)) { + for (const child of node) walk(child); + return; + } + if (Array.isArray(node.marks)) { + for (const mark of node.marks) { + if ( + mark && + mark.type === "link" && + mark.attrs && + isInternalPagePath(mark.attrs.href) + ) { + mark.attrs = { ...mark.attrs, ...INTERNAL_LINK_ATTRS }; + } + } + } + if (Array.isArray(node.content)) walk(node.content); +} diff --git a/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts b/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts index 6e1feac0..f6ce4696 100644 --- a/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts +++ b/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts @@ -12,6 +12,7 @@ import { parseHtmlDocument, generateJsonWith } from "./dom-parser.js"; import type { TokenizerExtension, RendererExtension } from "marked"; import { docmostExtensions } from "./docmost-schema.js"; import { parseAttachedComment } from "./attached-comment.js"; +import { markInternalLinks } from "./internal-links.js"; import { splitFootnoteParagraphs } from "./footnote.js"; import { decodeInlineMathLatex, @@ -1094,7 +1095,12 @@ export function markdownToProseMirrorSync(markdownContent: string): any { const withFootnotes = assembleFootnotes(withAttrs); const bridged = bridgeTaskLists(withFootnotes); const doc = generateJsonWith(bridged, docmostExtensions); - return stripEmptyParagraphs(doc); + // Promote unambiguously-internal wiki-page links (`[t](/s//p/)`) + // to their native internal form (`internal:true, target:null, rel:null`) so + // they get same-tab SPA navigation, hover-preview, and backlink participation. + // Every markdown import path funnels through here, so all of them are fixed at + // once; external links are left untouched (#522). + return markInternalLinks(stripEmptyParagraphs(doc)); } /** diff --git a/packages/prosemirror-markdown/test/internal-links.test.ts b/packages/prosemirror-markdown/test/internal-links.test.ts new file mode 100644 index 00000000..c330faa2 --- /dev/null +++ b/packages/prosemirror-markdown/test/internal-links.test.ts @@ -0,0 +1,316 @@ +import { describe, expect, it } from 'vitest'; +import { + isInternalPagePath, + markInternalLinks, +} from '../src/lib/internal-links.js'; +import { markdownToProseMirrorSync } from '../src/lib/markdown-to-prosemirror.js'; +import { + canonicalizeContent, + docsCanonicallyEqual, +} from '../src/lib/canonicalize.js'; + +// The server's canonical INTERNAL_LINK_REGEX +// (apps/server/src/integrations/export/utils.ts). `isInternalPagePath` MUST be a +// strict subset: everything it accepts must also match this. We assert the +// subset relation directly so drift is caught mechanically. +const SERVER_INTERNAL_LINK_REGEX = + /^(https?:\/\/)?([^\/]+)?(\/s\/([^\/]+)\/)?p\/([a-zA-Z0-9-]+)\/?$/; + +// Walk a doc collecting every link mark (across all text nodes). +const linkMarks = (doc: any): any[] => { + const out: any[] = []; + const walk = (n: any): void => { + if (!n || typeof n !== 'object') return; + if (Array.isArray(n)) return n.forEach(walk); + if (Array.isArray(n.marks)) + for (const m of n.marks) if (m?.type === 'link') out.push(m); + if (Array.isArray(n.content)) walk(n.content); + }; + walk(doc); + return out; +}; + +describe('isInternalPagePath', () => { + const ACCEPT = [ + '/s/eng/p/abc123', + '/s/eng/p/abc123/', // trailing slash + '/s/My-Space/p/A-b-C-9', // hyphens + mixed case in both segments + '/s/x/p/z', + ]; + const REJECT = [ + 'https://example.com/p/x', // scheme + host + 'http://host/s/eng/p/abc', // scheme + host, even on the internal shape + '//host/s/eng/p/abc', // protocol-relative host + '/p/abc123', // space-less form (server does NOT backlink it) + 'p/abc123', // relative + 's/eng/p/abc123', // missing leading slash + '/s/eng/p/abc#section', // anchor + '/s/eng/p/abc?q=1', // query + '/s/eng/p/abc/extra', // extra segment + '/s//p/abc', // empty space segment + '/s/eng/p/', // empty slug + '/s/eng/p', // no slug at all + '/api/pages/abc', // other internal route + '/s/eng/x/abc', // wrong middle segment + '/s/a/b/p/abc', // space contains slash -> extra segment + '', + ]; + + it('accepts the space-qualified root-relative page path (+trailing slash)', () => { + for (const href of ACCEPT) + expect(isInternalPagePath(href), href).toBe(true); + }); + + it('rejects external URLs, ambiguous, and non-page forms', () => { + for (const href of REJECT) + expect(isInternalPagePath(href), href).toBe(false); + }); + + it('rejects non-string input (fail-toward-external)', () => { + for (const v of [null, undefined, 123, {}, [], true]) + expect(isInternalPagePath(v as unknown)).toBe(false); + }); + + it('is a STRICT SUBSET of the server INTERNAL_LINK_REGEX', () => { + // Every accepted href must also satisfy the server regex (so it is + // guaranteed backlink-able / export-rewritable). + for (const href of ACCEPT) + expect(SERVER_INTERNAL_LINK_REGEX.test(href), href).toBe(true); + }); +}); + +describe('markInternalLinks', () => { + const linkNode = (text: string, href: string, extraMarks: any[] = []) => ({ + type: 'text', + text, + marks: [ + { + type: 'link', + attrs: { + href, + internal: null, + title: null, + target: '_blank', + rel: 'noopener noreferrer nofollow', + class: null, + }, + }, + ...extraMarks, + ], + }); + + it('marks an internal-path link internal:true, target:null, rel:null', () => { + const doc = { + type: 'doc', + content: [ + { type: 'paragraph', content: [linkNode('hi', '/s/eng/p/abc123')] }, + ], + }; + markInternalLinks(doc); + const m = linkMarks(doc)[0]; + expect(m.attrs.internal).toBe(true); + expect(m.attrs.target).toBeNull(); + expect(m.attrs.rel).toBeNull(); + expect(m.attrs.href).toBe('/s/eng/p/abc123'); // href untouched + }); + + it('leaves external links untouched', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [linkNode('ext', 'https://example.com/p/x')], + }, + ], + }; + markInternalLinks(doc); + const m = linkMarks(doc)[0]; + expect(m.attrs.internal).toBeNull(); + expect(m.attrs.target).toBe('_blank'); + expect(m.attrs.rel).toBe('noopener noreferrer nofollow'); + }); + + it('marks EVERY text node a multi-node link spans (incl. nested bold/italic)', () => { + // A link `[**bold** plain *ital*](/s/x/p/abc)` stores the link mark on each + // covered text node; some also carry bold/italic. All must become internal. + const bold = { type: 'bold' }; + const italic = { type: 'italic' }; + const doc = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + linkNode('bold', '/s/x/p/abc', [bold]), + linkNode(' plain ', '/s/x/p/abc'), + linkNode('ital', '/s/x/p/abc', [italic]), + ], + }, + ], + }; + markInternalLinks(doc); + const marks = linkMarks(doc); + expect(marks).toHaveLength(3); + for (const m of marks) { + expect(m.attrs.internal).toBe(true); + expect(m.attrs.target).toBeNull(); + expect(m.attrs.rel).toBeNull(); + } + // Nested bold/italic marks are preserved alongside the promoted link. + const nested = doc.content[0].content; + expect(nested[0].marks.some((m: any) => m.type === 'bold')).toBe(true); + expect(nested[2].marks.some((m: any) => m.type === 'italic')).toBe(true); + }); + + it('is idempotent', () => { + const doc = { + type: 'doc', + content: [ + { type: 'paragraph', content: [linkNode('hi', '/s/eng/p/abc')] }, + ], + }; + markInternalLinks(doc); + const once = JSON.stringify(doc); + markInternalLinks(doc); + expect(JSON.stringify(doc)).toBe(once); + }); +}); + +describe('markdown import (full converter) — #522 acceptance', () => { + it('promotes an internal link and leaves an external one external', () => { + const doc = markdownToProseMirrorSync( + '[t](/s/eng/p/abc123) and [ext](https://example.com/p/x)', + ); + const marks = linkMarks(doc); + const internal = marks.find((m) => m.attrs.href === '/s/eng/p/abc123'); + const external = marks.find( + (m) => m.attrs.href === 'https://example.com/p/x', + ); + expect(internal.attrs).toMatchObject({ + internal: true, + target: null, + rel: null, + }); + expect(external.attrs).toMatchObject({ + internal: null, + target: '_blank', + rel: 'noopener noreferrer nofollow', + }); + }); + + it('does NOT promote the space-less /p/ form (documented limit)', () => { + const doc = markdownToProseMirrorSync('[t](/p/abc123)'); + const m = linkMarks(doc)[0]; + expect(m.attrs.internal).toBeNull(); + expect(m.attrs.target).toBe('_blank'); + }); + + it('the promoted link is backlink-extractable (matches server regex + true)', () => { + // Mirrors extractInternalLinkSlugIds: internal flag AND server-regex match. + const doc = markdownToProseMirrorSync('[t](/s/eng/p/my-page-abc123)'); + const m = linkMarks(doc)[0]; + expect(m.attrs.internal).toBe(true); + const match = m.attrs.href.match(SERVER_INTERNAL_LINK_REGEX); + expect(match).not.toBeNull(); + // group 5 is the slug segment the server feeds to extractPageSlugId. + expect(match![5]).toBe('my-page-abc123'); + }); + + it('comment-body markdown gets the same treatment (shared converter path)', () => { + // Comment bodies go through the same markdownToProseMirrorSync; a spot-check + // that the shared path (not a comment-only branch) does the promotion. + const doc = markdownToProseMirrorSync('see [here](/s/team/p/xyz789)'); + const m = linkMarks(doc).find((x) => x.attrs.href === '/s/team/p/xyz789'); + expect(m.attrs.internal).toBe(true); + }); +}); + +describe('canonicalize — internal:false ≡ external (#522 §11)', () => { + it('drops editor-authored internal:false so external stays canonically equal', () => { + // Editor stores external links with internal:false; import leaves it absent. + const stored = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { + type: 'text', + text: 'x', + marks: [ + { + type: 'link', + attrs: { + href: 'https://example.com', + internal: false, + target: '_blank', + rel: 'noopener noreferrer nofollow', + }, + }, + ], + }, + ], + }, + ], + }; + const reimport = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { + type: 'text', + text: 'x', + marks: [ + { + type: 'link', + attrs: { + href: 'https://example.com', + internal: null, // import default + target: '_blank', + rel: 'noopener noreferrer nofollow', + }, + }, + ], + }, + ], + }, + ], + }; + expect(docsCanonicallyEqual(stored, reimport)).toBe(true); + // internal:false / target / rel all drop as defaults; only the non-default + // href survives. + const canon = canonicalizeContent(stored); + const mark = canon.content[0].content[0].marks[0]; + expect(mark.attrs).toEqual({ href: 'https://example.com' }); + }); + + it('keeps internal:true through canonicalization (non-default, load-bearing)', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { + type: 'text', + text: 'x', + marks: [ + { + type: 'link', + attrs: { href: '/s/x/p/abc', internal: true }, + }, + ], + }, + ], + }, + ], + }; + const canon = canonicalizeContent(doc); + const mark = canon.content[0].content[0].marks[0]; + expect(mark.attrs.internal).toBe(true); + expect(mark.attrs.href).toBe('/s/x/p/abc'); + }); +});