vvzvlad/gitmost
1
0
Fork 0
Code Issues 23 Pull Requests 7 Packages Projects Releases Activity

Merge pull request 'fix(prosemirror-markdown): markdown-импорт помечает внутренние ссылки на страницы как internal (#522)' (#524) from fix/522-internal-links into develop

Browse Source 
Reviewed-on: #524
This commit was merged in pull request #524.
This commit is contained in:
vvzvlad
2026-07-12 17:35:28 +03:00
parent af44736fb9 74387bb047
commit c30f910d66
6 changed files with 556 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/server/src/integrations/export/internal-link-parity.spec.ts
+70
View File
@@ -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([]);
});
});
packages/prosemirror-markdown/src/lib/canonicalize.ts
+18 -1
View File
@@ -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).
@@ -56,6 +65,14 @@ const KNOWN_DEFAULTS: Record<string, Record<string, unknown>> = {
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,
packages/prosemirror-markdown/src/lib/index.ts
+4
View File
@@ -38,6 +38,10 @@ export {
normalizeForeignMarkdown,
normalizeAgentMarkdown,
} from "./foreign-markdown.js";
// Pure primitive: detect the unambiguously-internal wiki-page link path
// (`/s/<space>/p/<slug>`) 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
packages/prosemirror-markdown/src/lib/internal-links.ts
+112
View File
@@ -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/<space>/p/<slugId>)`) 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/<space>/p/<slug>` (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/<slug>`, a relative
* `p/<slug>`, 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/<space>/p/<slug>` (with optional trailing slash). A STRICT SUBSET of the
* server's `INTERNAL_LINK_REGEX`:
* - `<space>` = `[^/]+` (any non-slash segment, as in the server's group 4)
* - `<slug>` = `[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<T>(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);
}
packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts
+7 -1
View File
@@ -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/<space>/p/<slug>)`)
// 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));
}
/**
packages/prosemirror-markdown/test/internal-links.test.ts
+345
View File
@@ -0,0 +1,345 @@
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';
// 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-]+)\/?$/;
// 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
'',
];
// Structurally VALID `/s/<space>/p/<slug>` 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)
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 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);
});
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/<slug> 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');
});
});