Merge remote-tracking branch 'gitea/develop' into fix/503-hardbreak-canon

Issue #503 reported a hardBreak sent via updatePageJson being «silently
cut» — no error, no line break — forcing the agent to split lines into
separate paragraphs.

A faithful repro shows the bug does NOT reproduce against develop:
hardBreak is fully representable in the markdown canon in BOTH directions
and survives the real Yjs write path.

- pm -> md: the serializer (now in @docmost/prosemirror-markdown after the
  #293 STEP 5 consolidation) emits the CommonMark hard break `  \n`, not a
  bare `\n`.
- md -> pm: marked tokenizes `  \n` to `<br>`, which generateJSON parses
  back to a hardBreak node in ONE paragraph (not split, not dropped).
- updatePageJson (PM JSON -> applyDocToFragment = PMNode.fromJSON +
  updateYFragment, the real collab-session write encoder -> read back)
  keeps text + hardBreak + text in one paragraph.

The issue's diagnosis points at packages/mcp/.../markdown-converter.ts as
the serializer, but that file is a 15-line re-export shim since #293 STEP 5
— the diagnosis predates the converter consolidation that already closed
both sides. P1 (semantic round-trip) + P2 (byte-fixpoint) hold, and the
node already has broad property/corpus/golden coverage in the package.

No converter change is warranted (switching the break form to `\`+newline
would churn the whole #351 byte-fixpoint corpus against the established
`  \n` repo convention for zero functional gain). This adds one
integration guard at the MCP canon seam covering all three seams of the
reported updatePageJson path. Mutation-verified: neutering the serializer
arm or the importer `<br>` handling reddens seams 1-2; neutering the real
applyDocToFragment write path reddens seam 3.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/server/src/core/page/page.controller.ts

@@ -818,11 +818,6 @@ export class PageController {
       throw new NotFoundException('Page not found');
     }
 
-    // Target-only validateCanView is intentional: getPageBreadCrumbs returns
-    // the full ancestor chain WITHOUT per-ancestor permission filtering. Safe
-    // because page restrictions inherit down the tree, so any ancestor the
-    // caller could not view would already hide the target here — see the
-    // getPageBreadCrumbs docstring / #471.
     await this.pageAccessService.validateCanView(page, user);
 
     return this.pageService.getPageBreadCrumbs(page.id);

apps/server/src/core/page/services/page.service.ts

@@ -1071,29 +1071,6 @@ export class PageService {
     });
   }
 
-  /**
-   * Walk the ancestor chain of `childPageId` up to the space root, filtered
-   * ONLY by `deletedAt` (+ MAX_PAGE_TREE_DEPTH) — WITHOUT per-ancestor
-   * permission filtering. Callers that expose this to a user (the
-   * `/breadcrumbs` endpoint) validate `validateCanView` on the TARGET page
-   * only, then return the whole chain of ancestor titles (#471).
-   *
-   * This is safe — NOT a title leak — because page restrictions inherit DOWN
-   * the tree: to view a page the caller must hold permission on EVERY
-   * restricted ancestor (`validateCanView` -> `canUserAccessPage` checks the
-   * full ancestor chain — see page-access.service.ts / page-permission.repo.ts
-   * `canUserEditPage`). A restricted ancestor the caller may not see would
-   * therefore already hide the TARGET page itself, so every ancestor reachable
-   * here is one the caller is already entitled to view (content stays gated
-   * regardless — getPage/getNode re-check permissions).
-   *
-   * Note the guarantee is the narrow "may view a descendant => may view its
-   * ancestors", NOT "space membership sees every page" — restricted subtrees do
-   * hide pages from members. Per-ancestor permission filtering here was
-   * considered and declined as redundant given the inheritance invariant above
-   * (#471). The same chain feeds the web-UI breadcrumb bar under identical CASL
-   * scope.
-   */
   async getPageBreadCrumbs(childPageId: string, trx?: KyselyTransaction) {
     const ancestors = await dbOrTx(this.db, trx)
       .withRecursive('page_ancestors', (db) =>

packages/mcp/test/unit/docmost-md-roundtrip.test.mjs

@@ -5,8 +5,13 @@ import {
   serializeDocmostMarkdown,
   parseDocmostMarkdown,
 } from "../../build/lib/markdown-document.js";
+import * as Y from "yjs";
 import { convertProseMirrorToMarkdown } from "../../build/lib/markdown-converter.js";
-import { markdownToProseMirror } from "../../build/lib/collaboration.js";
+import {
+  markdownToProseMirror,
+  applyDocToFragment,
+} from "../../build/lib/collaboration.js";
+import { TiptapTransformer } from "@hocuspocus/transformer";
 
 /** Recursively find the first descendant node (or self) of the given type. */
 function find(node, type) {
@@ -224,3 +229,68 @@ test("drawio round-trips through export and import", () => {
     assert.equal(diagram.attrs.attachmentId, "att-7");
   });
 });
+
+// #503: a hardBreak (Shift+Enter) must be REPRESENTABLE end-to-end through the
+// MCP canon, not silently cut. The reported symptom was a hardBreak sent via
+// updatePageJson vanishing with no error and no line break. This guards all
+// three seams of that path at once:
+//   1. pm -> md: the serializer emits the CommonMark hard break `  \n` (two
+//      trailing spaces), not a bare `\n` (which reimports as a soft break and is
+//      lost).
+//   2. md -> pm: `  \n` reimports to ONE paragraph carrying text+hardBreak+text
+//      (not two paragraphs, break not dropped) — the "непредставим" arm.
+//   3. the literal updatePageJson mechanism: PM JSON -> live Yjs fragment via
+//      `applyDocToFragment` (PMNode.fromJSON + updateYFragment — the SAME encoder
+//      the collab-session write path uses, NOT buildYDoc/toYdoc) -> read back
+//      preserves the hardBreak between the two text runs in ONE paragraph.
+test("hardBreak survives the MCP canon in BOTH directions (#503)", async () => {
+  const doc = {
+    type: "doc",
+    content: [
+      {
+        type: "paragraph",
+        content: [
+          { type: "text", text: "line one" },
+          { type: "hardBreak" },
+          { type: "text", text: "line two" },
+        ],
+      },
+    ],
+  };
+
+  // 1. pm -> md: exact CommonMark hard break, not a dropped/soft break.
+  const body = convertProseMirrorToMarkdown(doc);
+  assert.equal(body, "line one  \nline two");
+
+  // 2. md -> pm: ONE paragraph, text + hardBreak + text preserved (not split).
+  const rebuilt = await markdownToProseMirror(body);
+  const paras = findAll(rebuilt, "paragraph");
+  assert.equal(paras.length, 1, "the break must NOT split the paragraph in two");
+  const inline = paras[0].content.map((n) => n.type);
+  assert.deepEqual(
+    inline,
+    ["text", "hardBreak", "text"],
+    "the hardBreak must survive the md -> pm import",
+  );
+
+  // 2b. byte-fixpoint: a second export is stable (P2).
+  assert.equal(convertProseMirrorToMarkdown(rebuilt), body);
+
+  // 3. updatePageJson path: PM JSON -> live Yjs fragment (applyDocToFragment =
+  // the real collab write path's PMNode.fromJSON + updateYFragment) -> read back
+  // keeps text + hardBreak + text in ONE paragraph.
+  const ydoc = new Y.Doc();
+  applyDocToFragment(ydoc, doc);
+  const fromYjs = TiptapTransformer.fromYdoc(ydoc, "default");
+  const yjsParas = findAll(fromYjs, "paragraph");
+  assert.equal(
+    yjsParas.length,
+    1,
+    "the Yjs write path must NOT split the paragraph in two",
+  );
+  assert.deepEqual(
+    yjsParas[0].content.map((n) => n.type),
+    ["text", "hardBreak", "text"],
+    "the hardBreak must survive the updatePageJson (updateYFragment) write path",
+  );
+});