From 6ec2981743193865a118f9c51198312403e422e0 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 00:36:34 +0300 Subject: [PATCH 1/2] =?UTF-8?q?fix(prosemirror-markdown):=20=D0=BF=D0=BE?= =?UTF-8?q?=D0=BC=D0=B5=D1=87=D0=B0=D1=82=D1=8C=20=D0=B2=D0=BD=D1=83=D1=82?= =?UTF-8?q?=D1=80=D0=B5=D0=BD=D0=BD=D0=B8=D0=B5=20=D1=81=D1=81=D1=8B=D0=BB?= =?UTF-8?q?=D0=BA=D0=B8=20=D0=BD=D0=B0=20=D1=81=D1=82=D1=80=D0=B0=D0=BD?= =?UTF-8?q?=D0=B8=D1=86=D1=8B=20=D0=BA=D0=B0=D0=BA=20internal=20=D0=BF?= =?UTF-8?q?=D1=80=D0=B8=20=D0=B8=D0=BC=D0=BF=D0=BE=D1=80=D1=82=D0=B5=20mar?= =?UTF-8?q?kdown?= 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 | 4 + .../src/lib/internal-links.ts | 112 +++++++ .../src/lib/markdown-to-prosemirror.ts | 8 +- .../test/internal-links.test.ts | 316 ++++++++++++++++++ 5 files changed, 447 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 8bdfeb14..6bcd3ef0 100644 --- a/packages/prosemirror-markdown/src/lib/index.ts +++ b/packages/prosemirror-markdown/src/lib/index.ts @@ -38,6 +38,10 @@ export { normalizeForeignMarkdown, normalizeAgentMarkdown, } from "./foreign-markdown.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 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'); + }); +}); From 74387bb0470e4023e59f6366027e2b7a77558aee Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 03:40:18 +0300 Subject: [PATCH 2/2] test(#522): make the internal-link subset invariant a real cross-package drift guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer follow-up on the #522 internal-link import fix. The fix itself is confirmed correct; these three changes harden its load-bearing subset invariant and fix a stale doc. Finding 1 (drift guard): the in-package test only compared isInternalPagePath against a HAND-COPIED server regex, so a later narrowing of the real server INTERNAL_LINK_REGEX would leave the test green while the client silently became WIDER than the server. Add apps/server/.../export/internal-link-parity.spec.ts: the top layer already depends on @docmost/prosemirror-markdown and exports isInternalPagePath, so this spec imports the LIVE server INTERNAL_LINK_REGEX AND the LIVE isInternalPagePath and asserts subset over an accept-corpus — a server narrowing now reddens CI. The in-package manual copy is demoted from its "drift guard" role (comment updated to say so; it stays as local documentation). Finding 2 (charset non-vacuity): the REJECT list was purely structural, so the mutation widening the slug charset [a-zA-Z0-9-] -> [a-zA-Z0-9-.] survived the whole suite. Add REJECT cases whose ONLY defect is a forbidden slug character (abc.def / abc_def / abc%20 / abc~x); assert both isInternalPagePath and the server regex reject them. The mutation now reddens the new reject test. Finding 3 (stale JSDoc): canonicalize.ts KNOWN_DEFAULTS module blurb claimed every entry was read from docmost-schema and import-materialized. The new link.internal default breaks both claims (source is editor-ext/src/lib/link.ts; external links leave internal absent/null, never false). Add a bullet documenting the one editor-sourced, non-materialized default (false ≡ absent/null). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../export/internal-link-parity.spec.ts | 70 +++++++++++++++++++ .../src/lib/canonicalize.ts | 11 ++- .../test/internal-links.test.ts | 37 ++++++++-- 3 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 apps/server/src/integrations/export/internal-link-parity.spec.ts diff --git a/apps/server/src/integrations/export/internal-link-parity.spec.ts b/apps/server/src/integrations/export/internal-link-parity.spec.ts new file mode 100644 index 00000000..e08de660 --- /dev/null +++ b/apps/server/src/integrations/export/internal-link-parity.spec.ts @@ -0,0 +1,70 @@ +import { INTERNAL_LINK_REGEX } from './utils'; +import { isInternalPagePath } from '@docmost/prosemirror-markdown'; + +/** + * Cross-package DRIFT GUARD for the internal-link subset invariant (#522). + * + * The client-side `isInternalPagePath` + * (`packages/prosemirror-markdown/src/lib/internal-links.ts`) promotes a markdown + * link to `internal: true` on import. Every link it marks internal MUST be one + * the server would backlink and export-rewrite — i.e. the client matcher MUST be + * a STRICT SUBSET of the server's canonical `INTERNAL_LINK_REGEX` + * (`./utils.ts`). If the client ever accepts a path the server rejects, that link + * is stored internal but silently dropped from the backlink graph and broken on + * export — the exact bug #522 fixed. + * + * This spec is the load-bearing guard: it imports the LIVE server regex AND the + * LIVE `isInternalPagePath` (no hand-copied regex on either side). A narrowing of + * EITHER — most dangerously the server regex — reddens here. The in-package + * accept/reject test documents the client's behaviour but cannot see the server + * regex; this top-layer spec is what makes the subset relation mechanical + * (AGENTS.md rule #7: a CI test that fails on drift of the source of truth). + */ +describe('internal-link subset parity (client isInternalPagePath ⊆ server INTERNAL_LINK_REGEX)', () => { + // Every path the CLIENT accepts. Kept deliberately broad across the risky + // dimensions — the slug charset (digits, hyphens, mixed case, leading/trailing + // hyphen), the space charset, and the optional trailing slash — so a narrowing + // of the server regex on any of them reddens the subset assertion below. + const CLIENT_ACCEPTS = [ + '/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', // shortest + '/s/eng/p/my-page-abc123', // slug with hyphens (extractPageSlugId shape) + '/s/eng/p/-lead', // leading hyphen in slug + '/s/eng/p/trail-', // trailing hyphen in slug + '/s/eng/p/0123456789', // all-digit slug + '/s/a.b/p/abc', // dot in the SPACE segment (space charset is [^/]+) + '/s/space with space/p/abc', // space char in the SPACE segment + ]; + + it('every corpus path is actually client-accepted (guards the corpus itself)', () => { + // If a path here stopped being client-accepted the subset test would pass + // vacuously; assert acceptance up front so the corpus stays meaningful. The + // filter-to-empty form names the offending paths on failure. + const notAccepted = CLIENT_ACCEPTS.filter((h) => !isInternalPagePath(h)); + expect(notAccepted).toEqual([]); + }); + + it('every client-accepted path also matches the LIVE server regex (subset)', () => { + // The mechanical drift guard: narrow the server INTERNAL_LINK_REGEX and at + // least one hyphen/charset/structure case appears here. + const notInServer = CLIENT_ACCEPTS.filter((h) => !INTERNAL_LINK_REGEX.test(h)); + expect(notInServer).toEqual([]); + }); + + it('the client rejects forbidden-slug-char paths the server also rejects', () => { + // Documents the subset BOUNDARY: correct shape, forbidden slug char. The + // client and the LIVE server regex must agree on rejection. + const forbidden = [ + '/s/eng/p/abc.def', + '/s/eng/p/abc_def', + '/s/eng/p/abc%20', + '/s/eng/p/abc~x', + ]; + const clientAccepts = forbidden.filter((h) => isInternalPagePath(h)); + const serverAccepts = forbidden.filter((h) => INTERNAL_LINK_REGEX.test(h)); + expect(clientAccepts).toEqual([]); + expect(serverAccepts).toEqual([]); + }); +}); diff --git a/packages/prosemirror-markdown/src/lib/canonicalize.ts b/packages/prosemirror-markdown/src/lib/canonicalize.ts index b1f7593e..39896132 100644 --- a/packages/prosemirror-markdown/src/lib/canonicalize.ts +++ b/packages/prosemirror-markdown/src/lib/canonicalize.ts @@ -25,13 +25,22 @@ * drop any attr whose value equals its known schema default. A non-default * value (e.g. `orderedList.start: 5`) is NOT a default, so it is KEPT. * - * Every entry below was read from `packages/docmost-client/src/lib/ + * Every entry below — with the single documented exception of the `link.internal` + * marker (see its own bullet) — was read from `packages/docmost-client/src/lib/ * docmost-schema.ts` (the line refs are the exact `default:` declarations) and * confirmed to be materialized by an export→import→export round-trip: * - mark `link` target / rel — DocmostAttributes + StarterKit link. * StarterKit's link extension defaults `target: "_blank"` and * `rel: "noopener noreferrer nofollow"`; both materialize on import * (empirically confirmed) even when the source had only `href`. + * - mark `link` internal (#522) — the ONE editor-sourced, NOT-import- + * materialized default here. Its source is `editor-ext/src/lib/link.ts` + * (default `internal: false`), not docmost-schema, and import does NOT + * materialize it (an imported external link leaves `internal` absent/null, + * never `false`). It is listed so that the editor's stored `internal:false` + * normalizes to the same "external" canon as absent/null (`false ≡ absent`), + * keeping a stored external link canonically equal to its re-import. The + * load-bearing `internal:true` is NON-default and therefore KEPT. * - mark `comment` resolved — docmost-schema.ts L213-214 (`default: false`). * - node `orderedList` start — provided by StarterKit's orderedList * (`default: 1`); materializes on import (empirically confirmed). diff --git a/packages/prosemirror-markdown/test/internal-links.test.ts b/packages/prosemirror-markdown/test/internal-links.test.ts index c330faa2..19c1037d 100644 --- a/packages/prosemirror-markdown/test/internal-links.test.ts +++ b/packages/prosemirror-markdown/test/internal-links.test.ts @@ -9,10 +9,14 @@ import { 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. +// A LOCAL, illustrative copy of the server's INTERNAL_LINK_REGEX +// (apps/server/src/integrations/export/utils.ts) used only to document the +// subset boundary WITHIN this package (this layer cannot import the server). +// It is NOT the cross-package drift guard: a hand copy can silently go stale if +// the server regex is later narrowed. The real, mechanical drift guard lives in +// the top layer, `apps/server/src/integrations/export/internal-link-parity.spec.ts`, +// which imports BOTH the LIVE server `INTERNAL_LINK_REGEX` and the LIVE +// `isInternalPagePath` and reddens if the client ever ceases to be a subset. const SERVER_INTERNAL_LINK_REGEX = /^(https?:\/\/)?([^\/]+)?(\/s\/([^\/]+)\/)?p\/([a-zA-Z0-9-]+)\/?$/; @@ -55,6 +59,18 @@ describe('isInternalPagePath', () => { '/s/a/b/p/abc', // space contains slash -> extra segment '', ]; + // Structurally VALID `/s//p/` shapes whose ONLY defect is a + // forbidden character in the slug segment. These pin the slug charset + // `[a-zA-Z0-9-]` itself (not just the surrounding structure): drop them and a + // mutation widening the charset (e.g. adding `.`) survives the suite. Each is + // ALSO rejected by the server regex — documenting that the subset boundary + // holds precisely on the charset, not just on structure. + const REJECT_SLUG_CHARSET = [ + '/s/eng/p/abc.def', // dot + '/s/eng/p/abc_def', // underscore + '/s/eng/p/abc%20', // percent-encoded space + '/s/eng/p/abc~x', // tilde + ]; it('accepts the space-qualified root-relative page path (+trailing slash)', () => { for (const href of ACCEPT) @@ -66,6 +82,19 @@ describe('isInternalPagePath', () => { expect(isInternalPagePath(href), href).toBe(false); }); + it('rejects a correctly-shaped path with a forbidden slug character', () => { + // Pins the slug charset itself: a mutation widening `[a-zA-Z0-9-]` reddens. + for (const href of REJECT_SLUG_CHARSET) + expect(isInternalPagePath(href), href).toBe(false); + }); + + it('the forbidden-slug-char paths are rejected by the server regex too', () => { + // The subset boundary holds on the charset, not just the structure: none of + // these match the server regex, so the client must not accept them either. + for (const href of REJECT_SLUG_CHARSET) + expect(SERVER_INTERNAL_LINK_REGEX.test(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);