diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index d3c62050..41cb38d8 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -42,6 +42,7 @@ import { insertTableRow, deleteTableRow, updateTableCell, + findInvalidNode, } from "@docmost/prosemirror-markdown"; import { searchInDoc, SearchOptions } from "./lib/page-search.js"; import { withPageLock } from "./lib/page-lock.js"; @@ -1660,6 +1661,27 @@ export class DocmostClient { } } + /** + * Pre-write SHAPE gate (#409). Walk the WHOLE node tree with the shared + * `findInvalidNode` and throw a rich, path-anchored error the instant a nested + * node has an absent/unknown `type` (or an unknown mark) — the exact shape that + * otherwise surfaces DEEP in the Yjs encode as the cryptic + * `Unknown node type: undefined`, but only AFTER a collab session was opened + * and a page lock taken. Calling this BEFORE `getCollabTokenWithReauth` / + * `mutatePageContent` fails fast: no collab connection, no lock, deterministic + * message. `op` names the tool for the message prefix (e.g. "patch_node"). + * + * `findInvalidNode` derives its "known type" set from the very same + * `docmostExtensions` the encode path uses, so a node this gate accepts is one + * the encoder will accept too. + */ + private assertValidNodeShape(op: string, node: any): void { + const bad = findInvalidNode(node); + if (bad) { + throw new Error(`${op}: invalid node — ${bad.summary}`); + } + } + /** * Replace page content with a raw ProseMirror JSON document (lossless) and/or * update its title. Both `doc` and `title` are optional, but at least one must @@ -1711,6 +1733,12 @@ export class DocmostClient { // page on overwrite. this.validateDocStructure(doc); + // #409: beyond the string-`type` check above, reject a nested node whose + // `type` is a string but NOT a known Docmost schema node (a typo/unknown + // block) — the same `Unknown node type` the encoder throws — with a rich, + // path-anchored message, still BEFORE any collab connection. + this.assertValidNodeShape("update_page_json", doc); + // Sanitize URLs before writing. This closes the JSON-path bypass: unlike // the markdown link path (which TipTap sanitizes), raw JSON could otherwise // inject javascript:/data: link hrefs or media srcs straight into the doc. @@ -2131,6 +2159,14 @@ export class DocmostClient { target.attrs.id = nodeId; } + // #409: fail fast on a malformed node SHAPE (a nested child with an + // absent/unknown `type`, e.g. a text leaf written as `{"text":"foo"}` with + // no `"type":"text"`) BEFORE opening a collab session or taking the page + // lock — the root-only `typeof node.type === "string"` check above never + // sees nested children, and the encoder's `Unknown node type: undefined` + // would otherwise only surface after the connection. + this.assertValidNodeShape("patch_node", target); + const collabToken = await this.getCollabTokenWithReauth(); // Open the collab doc by the canonical UUID, never the slugId (#260). const pageUuid = await this.resolvePageId(pageId); @@ -2221,6 +2257,11 @@ export class DocmostClient { } } + // #409: fail fast on a malformed node SHAPE (a nested child with an + // absent/unknown `type`) BEFORE opening a collab session or taking the page + // lock — the root-only check above never sees nested children. + this.assertValidNodeShape("insert_node", node); + const collabToken = await this.getCollabTokenWithReauth(); // Open the collab doc by the canonical UUID, never the slugId (#260). const pageUuid = await this.resolvePageId(pageId); diff --git a/packages/mcp/src/lib/collaboration.ts b/packages/mcp/src/lib/collaboration.ts index bbd805b2..559d7519 100644 --- a/packages/mcp/src/lib/collaboration.ts +++ b/packages/mcp/src/lib/collaboration.ts @@ -13,7 +13,11 @@ import { JSDOM } from "jsdom"; import { markdownToProseMirror } from "@docmost/prosemirror-markdown"; import { docmostExtensions, docmostSchema } from "./docmost-schema.js"; import { withPageLock } from "./page-lock.js"; -import { sanitizeForYjs, findUnstorableAttr } from "@docmost/prosemirror-markdown"; +import { + sanitizeForYjs, + findUnstorableAttr, + findInvalidNode, +} from "@docmost/prosemirror-markdown"; import { canonicalizeFootnotes } from "./footnote-canonicalize.js"; import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js"; import { VerifyReport } from "./diff.js"; @@ -28,11 +32,25 @@ export { markdownToProseMirror }; * place. `label` names the stage that failed (diagnostic). `sanitizeForYjs` * already stripped `undefined` attrs, so a remaining failure is pinpointed via * `findUnstorableAttr`. + * + * Diagnostics precedence (#409): the dominant crash here is + * `Unknown node type: undefined` — a nested node with an absent/unknown `type` + * (a SHAPE problem, e.g. `{"text":"foo"}` missing `"type":"text"`). That points + * at the node, not an attribute, so `findInvalidNode` is consulted FIRST and, + * on a hit, yields a path-anchored node-shape message. Only when the document + * shape is sound do we fall back to `findUnstorableAttr` (undefined/function/ + * symbol/bigint attr values); the generic "attribute likely holds a value Yjs + * cannot store" sentence is the last resort. */ function unstorableYjsError(safe: any, label: string, e: unknown): Error { + const base = `Failed to encode document to Yjs (${label}): ${e instanceof Error ? e.message : String(e)}.`; + const badNode = findInvalidNode(safe); + if (badNode) { + return new Error(`${base} Invalid node: ${badNode.summary}`); + } const bad = findUnstorableAttr(safe); return new Error( - `Failed to encode document to Yjs (${label}): ${e instanceof Error ? e.message : String(e)}.${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`, + `${base}${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`, ); } diff --git a/packages/mcp/src/tool-specs.ts b/packages/mcp/src/tool-specs.ts index 1516f62e..66243653 100644 --- a/packages/mcp/src/tool-specs.ts +++ b/packages/mcp/src/tool-specs.ts @@ -236,7 +236,10 @@ export const SHARED_TOOL_SPECS = { 'heading {"type":"heading","attrs":{"level":2},"content":' + '[{"type":"text","text":"Title"}]}. Bold is a mark: ' + '{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' + - 'JSON object or a JSON string (both accepted). Cheaper and safer than ' + + 'JSON object or a JSON string (both accepted). EVERY node, including ' + + 'nested children, must carry a string `type` from the Docmost schema; ' + + 'text leaves are {"type":"text","text":"..."} (a bare {"text":"..."} is ' + + 'rejected up front). Cheaper and safer than ' + 'replacing the whole document for one-block structural edits. Reversible: ' + 'the previous version is kept in page history.', tier: 'deferred', @@ -282,7 +285,10 @@ export const SHARED_TOOL_SPECS = { '{"type":"paragraph","content":[{"type":"text","text":"Hello"}]} or a ' + 'heading {"type":"heading","attrs":{"level":2},"content":' + '[{"type":"text","text":"Title"}]}. Bold is a mark: ' + - '{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' + + '{"type":"text","text":"x","marks":[{"type":"bold"}]}. EVERY node, ' + + 'including nested children, must carry a string `type` from the Docmost ' + + 'schema; text leaves are {"type":"text","text":"..."} (a bare ' + + '{"text":"..."} is rejected up front). The node may be a ' + 'JSON object or a JSON string (both accepted). Reversible via page history.', tier: 'deferred', catalogLine: @@ -705,7 +711,10 @@ export const SHARED_TOOL_SPECS = { '"paragraph","content":[{"type":"text","text":"Hi"}]}]}. `content` may be ' + 'a JSON object or a JSON string (both accepted), and is OPTIONAL: omit it ' + 'to update only the title (though prefer the rename-page tool for a title-only ' + - 'change). Supplying neither content nor title is an error. Reversible: ' + + 'change). Supplying neither content nor title is an error. EVERY node, ' + + 'including nested children, must carry a string `type` from the Docmost ' + + 'schema; text leaves are {"type":"text","text":"..."} (a bare ' + + '{"text":"..."} is rejected up front). Reversible: ' + 'the previous version is kept in page history.', tier: 'deferred', catalogLine: diff --git a/packages/mcp/test/mock/invalid-node-validation.test.mjs b/packages/mcp/test/mock/invalid-node-validation.test.mjs new file mode 100644 index 00000000..3e59c805 --- /dev/null +++ b/packages/mcp/test/mock/invalid-node-validation.test.mjs @@ -0,0 +1,240 @@ +// Mock regression for the FAIL-FAST invalid-node validation (#409). +// +// A structural editor (patch_node / insert_node / update_page_json) given a doc +// whose NESTED child has an absent/unknown `type` (the exact shape the Yjs +// encoder rejects with `Unknown node type: undefined`) must throw a RICH, +// path-anchored error BEFORE it ever opens a collab session or takes a page +// lock. We prove the fail-fast by standing up a collab stack whose HTTP handler +// records EVERY request: a correct fail-fast never even fetches the collab +// token (which `getCollabTokenWithReauth`, called AFTER the validation, would +// request), and never drives a document change on the Hocuspocus doc. +// +// The happy path (a well-formed doc) is exercised too: it must reach the collab +// write and succeed, so the gate is not over-eager. +// +// findInvalidNode's per-shape summaries are unit-tested in the package +// (test/find-invalid-node.test.ts); this exercises the END-TO-END wiring through +// the real client methods. +import { test, after } from "node:test"; +import assert from "node:assert/strict"; +import http from "node:http"; +import { WebSocketServer } from "ws"; +import { Hocuspocus } from "@hocuspocus/server"; +import { DocmostClient } from "../../build/client.js"; +import { buildYDoc } from "../../build/lib/collaboration.js"; + +// A minimal valid seed doc with a real block id, so the happy-path patch_node +// finds its target. +const SEED_ID = "seed-para-id"; +function seedDoc() { + return { + type: "doc", + content: [ + { + type: "paragraph", + attrs: { id: SEED_ID }, + content: [{ type: "text", text: "seed" }], + }, + ], + }; +} + +// Stand up an HTTP server that authenticates + hands out a collab token AND +// upgrades /collab to a Hocuspocus instance seeded with the doc. `state` records +// whether the collab token was ever fetched (proving the write path was entered) +// and whether the Hocuspocus doc ever changed. +async function spawnCollabStack() { + const state = { changed: false, collabTokenFetched: false }; + + const hocuspocus = new Hocuspocus({ + quiet: true, + async onLoadDocument() { + return buildYDoc(seedDoc()); + }, + async onChange() { + state.changed = true; + }, + }); + + const wss = new WebSocketServer({ noServer: true }); + + const server = http.createServer((req, res) => { + let raw = ""; + req.on("data", (c) => (raw += c)); + req.on("end", () => { + if (req.url === "/api/auth/login") { + res.writeHead(200, { + "Content-Type": "application/json", + "Set-Cookie": "authToken=t; Path=/; HttpOnly", + }); + res.end(JSON.stringify({ success: true })); + return; + } + if (req.url === "/api/auth/collab-token") { + state.collabTokenFetched = true; + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ data: { token: "collab-jwt" } })); + return; + } + res.writeHead(404, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ message: "not found" })); + }); + }); + + server.on("upgrade", (request, socket, head) => { + if (!request.url || !request.url.startsWith("/collab")) { + socket.destroy(); + return; + } + wss.handleUpgrade(request, socket, head, (ws) => { + hocuspocus.handleConnection(ws, request); + }); + }); + + const baseURL = await new Promise((resolve) => { + server.listen(0, "127.0.0.1", () => { + const { port } = server.address(); + resolve(`http://127.0.0.1:${port}/api`); + }); + }); + + openStacks.push({ server, hocuspocus }); + return { state, baseURL }; +} + +const openStacks = []; +after(async () => { + await Promise.all( + openStacks.map( + ({ server, hocuspocus }) => + new Promise((resolve) => { + server.close(() => { + Promise.resolve(hocuspocus.destroy?.()).finally(resolve); + }); + }), + ), + ); +}); + +const PAGE = "11111111-1111-4111-8111-111111111111"; + +// A node whose NESTED text leaf is missing "type":"text" (dominant #409 shape). +const nestedTypelessNode = () => ({ + type: "paragraph", + content: [{ text: "oops", marks: [] }], +}); + +// A node with a NESTED unknown type NAME (typo). +const nestedUnknownTypeNode = () => ({ + type: "paragraph", + content: [{ type: "paragraf", content: [{ type: "text", text: "x" }] }], +}); + +test("patch_node fails fast on a nested typeless node — no collab connection", async () => { + const { state, baseURL } = await spawnCollabStack(); + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + await assert.rejects( + () => client.patchNode(PAGE, SEED_ID, nestedTypelessNode()), + (err) => { + assert.match(err.message, /patch_node: invalid node/); + assert.match(err.message, /missing "type"/); + assert.match(err.message, /content\[0\]/); // path-anchored + return true; + }, + ); + + assert.equal( + state.collabTokenFetched, + false, + "must NOT fetch a collab token — validation runs before getCollabTokenWithReauth", + ); + assert.equal(state.changed, false, "the collab doc must never be written"); +}); + +test("insert_node fails fast on a nested UNKNOWN type — no collab connection", async () => { + const { state, baseURL } = await spawnCollabStack(); + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + await assert.rejects( + () => + client.insertNode(PAGE, nestedUnknownTypeNode(), { + position: "append", + }), + (err) => { + assert.match(err.message, /insert_node: invalid node/); + assert.match(err.message, /unknown node type "paragraf"/); + return true; + }, + ); + + assert.equal(state.collabTokenFetched, false); + assert.equal(state.changed, false); +}); + +test("update_page_json fails fast on a nested typeless node — no collab connection", async () => { + const { state, baseURL } = await spawnCollabStack(); + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + const badDoc = { + type: "doc", + content: [{ type: "paragraph", content: [{ text: "oops" }] }], + }; + + await assert.rejects( + () => client.updatePageJson(PAGE, badDoc), + (err) => { + // update_page_json runs validateDocStructure first (string-type check), + // which already rejects a typeless node — so the message may come from + // either guard, but the write must not happen. + assert.match(err.message, /type/i); + return true; + }, + ); + + assert.equal(state.collabTokenFetched, false); + assert.equal(state.changed, false); +}); + +test("update_page_json fails fast on a nested UNKNOWN type name — rich #409 message", async () => { + const { state, baseURL } = await spawnCollabStack(); + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + // validateDocStructure passes (type is a string); assertValidNodeShape must + // catch the unknown schema name and produce the rich path-anchored message. + const badDoc = { + type: "doc", + content: [{ type: "paragraf", content: [{ type: "text", text: "x" }] }], + }; + + await assert.rejects( + () => client.updatePageJson(PAGE, badDoc), + (err) => { + assert.match(err.message, /update_page_json: invalid node/); + assert.match(err.message, /unknown node type "paragraf"/); + return true; + }, + ); + + assert.equal(state.collabTokenFetched, false); + assert.equal(state.changed, false); +}); + +test("patch_node with a well-formed node proceeds to the collab write", async () => { + const { state, baseURL } = await spawnCollabStack(); + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + const result = await client.patchNode(PAGE, SEED_ID, { + type: "paragraph", + content: [{ type: "text", text: "replacement" }], + }); + + assert.equal(result.success, true); + assert.equal(result.replaced, 1); + assert.equal( + state.collabTokenFetched, + true, + "a valid node must reach the collab write path", + ); + assert.equal(state.changed, true, "the collab doc must be written"); +}); diff --git a/packages/mcp/test/unit/unstorable-yjs-error.test.mjs b/packages/mcp/test/unit/unstorable-yjs-error.test.mjs new file mode 100644 index 00000000..51d00a1c --- /dev/null +++ b/packages/mcp/test/unit/unstorable-yjs-error.test.mjs @@ -0,0 +1,82 @@ +// #409: unstorableYjsError diagnostics PRECEDENCE. The opaque Yjs encode failure +// (`Unknown node type: undefined`) is a node-SHAPE problem, so the shared +// findInvalidNode is consulted FIRST and yields a path-anchored node message; +// only a shape-sound doc falls back to findUnstorableAttr (undefined/function/ +// etc. attr values). Exercised through `assertYjsEncodable`, which runs the same +// encode + error-wrapping the live write path uses. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { assertYjsEncodable } from "../../build/lib/collaboration.js"; +import { + findInvalidNode, + findUnstorableAttr, +} from "@docmost/prosemirror-markdown"; + +const doc = (...content) => ({ type: "doc", content }); + +test("a nested typeless node yields the rich node-shape message (not an attr hint)", () => { + const bad = doc({ + type: "paragraph", + content: [{ text: "oops", marks: [] }], // missing "type":"text" + }); + assert.throws( + () => assertYjsEncodable(bad), + (err) => { + assert.match(err.message, /Invalid node:/); + assert.match(err.message, /missing "type"/); + assert.match(err.message, /content\[0\]/); + // It must NOT fall through to the generic attribute sentence. + assert.doesNotMatch(err.message, /Offending attribute/); + assert.doesNotMatch( + err.message, + /attribute likely holds a value Yjs cannot store/, + ); + return true; + }, + ); +}); + +test("PRECEDENCE: a genuine undefined-attr case is a node-SHAPE-clean case, so the attr fallback fires", () => { + // A doc whose node shapes are ALL valid but that carries a Yjs-unstorable + // undefined attribute. unstorableYjsError checks findInvalidNode FIRST (must + // miss here) and only then findUnstorableAttr (must hit) — this is exactly the + // division of labor that keeps a real attr problem from being mislabelled as a + // node-shape problem, and vice versa. We assert the two helpers directly (the + // wrapper is not exported) because sanitizeForYjs strips undefined attrs before + // the live encoder ever sees them, so this branch cannot be reached through + // assertYjsEncodable without also failing the clone. + const attrProblem = doc({ + type: "paragraph", + attrs: { id: "p1", indent: undefined }, + content: [{ type: "text", text: "hi" }], + }); + // findInvalidNode: shape is clean -> null (so the wrapper does NOT emit + // "Invalid node"). + assert.equal(findInvalidNode(attrProblem), null); + // findUnstorableAttr: pinpoints the undefined attr -> the fallback message. + assert.match(findUnstorableAttr(attrProblem) ?? "", /indent \(undefined\)/); +}); + +test("PRECEDENCE: a node-shape problem is caught by findInvalidNode even when an attr is also unstorable", () => { + // Both a shape problem (typeless nested leaf) AND an unstorable attr exist; + // findInvalidNode wins, so the model is pointed at the node shape (the real + // root cause of `Unknown node type: undefined`), not the attribute. + const both = doc({ + type: "paragraph", + attrs: { id: "p1", indent: undefined }, + content: [{ text: "oops" }], + }); + const shape = findInvalidNode(both); + assert.notEqual(shape, null); + assert.match(shape.summary, /missing "type"/); +}); + +test("a fully valid document encodes without throwing", () => { + const good = doc({ + type: "paragraph", + attrs: { id: "p1" }, + content: [{ type: "text", text: "hi" }], + }); + assert.doesNotThrow(() => assertYjsEncodable(good)); +}); diff --git a/packages/prosemirror-markdown/src/lib/index.ts b/packages/prosemirror-markdown/src/lib/index.ts index 3279dfc6..5cdbdb91 100644 --- a/packages/prosemirror-markdown/src/lib/index.ts +++ b/packages/prosemirror-markdown/src/lib/index.ts @@ -56,6 +56,7 @@ export { deleteNodeById, sanitizeForYjs, findUnstorableAttr, + findInvalidNode, insertNodeRelative, readTable, insertTableRow, diff --git a/packages/prosemirror-markdown/src/lib/node-ops.ts b/packages/prosemirror-markdown/src/lib/node-ops.ts index cdb67902..50d1d69f 100644 --- a/packages/prosemirror-markdown/src/lib/node-ops.ts +++ b/packages/prosemirror-markdown/src/lib/node-ops.ts @@ -14,7 +14,9 @@ * `content`, non-object nodes, and absent `attrs` are tolerated. */ +import { getSchema } from "@tiptap/core"; import { stripInlineMarkdown } from "./text-normalize.js"; +import { docmostExtensions } from "./docmost-schema.js"; /** Deep-clone a JSON-serializable value without mutating the original. */ function clone(value: T): T { @@ -383,6 +385,119 @@ export function findUnstorableAttr(doc: any): string | null { return null; } +/** + * The Docmost schema's known node and mark NAME sets, derived ONCE from the very + * same `docmostExtensions` the Yjs encode path builds its schema from + * (`getSchema(docmostExtensions)` — mirrored in mcp's `docmostSchema`). Deriving + * both from the same extension list guarantees `findInvalidNode`'s "known type" + * set matches exactly what `PMNode.fromJSON`/`toYdoc` will actually accept, so + * the walker never flags a node the encoder would have stored (or vice versa). + * Lazy + cached: the schema is only built on first use. + */ +let schemaNames: { nodes: Set; marks: Set } | null = null; +function getSchemaNames(): { nodes: Set; marks: Set } { + if (schemaNames == null) { + const schema = getSchema(docmostExtensions); + schemaNames = { + nodes: new Set(Object.keys(schema.nodes)), + marks: new Set(Object.keys(schema.marks)), + }; + } + return schemaNames; +} + +/** + * Depth-first walk of the JSON `content` tree looking for the FIRST node whose + * SHAPE the Yjs encode path will reject with an opaque + * `Unknown node type: undefined` (issue #409). Returns `{ path, summary }` for + * the offending node, or `null` when every node (and every mark) is a known + * Docmost schema type. + * + * Two failure modes are detected, in order, per node: + * 1. `type` is missing or not a string — the dominant `undefined` case, e.g. + * a text leaf written as `{"text":"foo"}` with no `"type":"text"`. + * 2. `type` is a string but NOT a known Docmost node name (a typo / unknown + * block), OR one of the node's marks carries an unknown mark name. + * + * The returned `summary` is a model-actionable, path-anchored message such as: + * `node.content[2].content[0]: missing "type" (keys: text, marks) — did you + * mean {"type": "text", ...}?` + * or for an unknown type: + * `node.content[1]: unknown node type "paragraf" — not in the Docmost schema` + * + * `path` is the same dotted JSON path used in the summary (e.g. + * `node.content[2].content[0]`) so callers can surface it separately. Null-safe: + * a non-object doc returns `null`. + * + * NOTE: This is a SHAPE check, not a full ProseMirror content-model validation + * (it does not verify that a paragraph may legally contain a table, etc.). Its + * job is to turn the specific "unknown/absent node type" Yjs crash into a clear, + * pre-write diagnostic; the schema's own `.check()` still catches deeper + * content-model violations at encode time. + */ +export function findInvalidNode( + doc: any, +): { path: string; summary: string } | null { + if (!isObject(doc)) return null; + const { nodes, marks } = getSchemaNames(); + + // Build the "did you mean" hint for a typeless node from its own keys, so the + // model sees WHICH object is malformed and the canonical text-leaf fix. + const keyHint = (node: Record): string => { + const keys = Object.keys(node); + const looksLikeText = + typeof node.text === "string" && node.type === undefined; + const suffix = looksLikeText + ? ` — did you mean {"type": "text", ...}?` + : ` — every node needs a string "type" from the Docmost schema`; + return `missing "type" (keys: ${keys.join(", ") || "none"})${suffix}`; + }; + + const walk = ( + node: any, + path: string, + ): { path: string; summary: string } | null => { + if (!isObject(node)) return null; + + // (1) missing / non-string type. + if (typeof node.type !== "string") { + return { path, summary: `${path}: ${keyHint(node)}` }; + } + // (2) string type that is not a known Docmost node. + if (!nodes.has(node.type)) { + return { + path, + summary: `${path}: unknown node type "${node.type}" — not in the Docmost schema`, + }; + } + // (2b) unknown mark on an otherwise-valid node. + if (Array.isArray(node.marks)) { + for (let i = 0; i < node.marks.length; i++) { + const mark = node.marks[i]; + if (isObject(mark) && typeof mark.type === "string" && !marks.has(mark.type)) { + return { + path: `${path}.marks[${i}]`, + summary: `${path}.marks[${i}]: unknown mark type "${mark.type}" — not in the Docmost schema`, + }; + } + } + } + + if (Array.isArray(node.content)) { + for (let i = 0; i < node.content.length; i++) { + const hit = walk(node.content[i], `${path}.content[${i}]`); + if (hit != null) return hit; + } + } + return null; + }; + + // The root doc node is addressed as "node" (matching the mcp arg name); its + // children are node.content[i]. The root itself is checked too so a typeless + // root is reported rather than silently skipped. + return walk(doc, "node"); +} + /** * Table structural node types and the container each must live directly inside. * Used by `insertNodeRelative` to splice rows/cells into the correct ancestor diff --git a/packages/prosemirror-markdown/test/find-invalid-node.test.ts b/packages/prosemirror-markdown/test/find-invalid-node.test.ts new file mode 100644 index 00000000..e60a144a --- /dev/null +++ b/packages/prosemirror-markdown/test/find-invalid-node.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from 'vitest'; +import { findInvalidNode } from '../src/lib/node-ops.js'; + +// findInvalidNode (#409): a depth-first SHAPE gate that turns the encoder's +// opaque `Unknown node type: undefined` into a path-anchored, pre-write +// diagnostic. It flags the FIRST node whose `type` is absent/non-string or not +// a known Docmost schema node, or that carries an unknown mark; returns null for +// a well-formed doc. + +const doc = (...content: any[]) => ({ type: 'doc', content }); +const para = (...content: any[]) => ({ + type: 'paragraph', + attrs: { id: 'p1' }, + content, +}); +const text = (value: string, marks?: any[]) => { + const node: any = { type: 'text', text: value }; + if (marks) node.marks = marks; + return node; +}; + +describe('findInvalidNode', () => { + it('returns null for a fully valid document', () => { + const good = doc( + para(text('hello ', [{ type: 'bold' }]), text('world')), + { + type: 'heading', + attrs: { id: 'h1', level: 2 }, + content: [text('Title')], + }, + ); + expect(findInvalidNode(good)).toBeNull(); + }); + + it('flags a NESTED typeless text leaf with a path-precise summary', () => { + // A text leaf written as {"text":"foo"} with no "type":"text" — the dominant + // `Unknown node type: undefined` cause. + const bad = doc( + para(text('ok')), + para({ text: 'foo', marks: [] } as any), + ); + const hit = findInvalidNode(bad); + expect(hit).not.toBeNull(); + // Second paragraph (index 1), first child (index 0). + expect(hit!.path).toBe('node.content[1].content[0]'); + expect(hit!.summary).toContain('node.content[1].content[0]'); + expect(hit!.summary).toContain('missing "type"'); + expect(hit!.summary).toContain('keys: text, marks'); + expect(hit!.summary).toContain('did you mean {"type": "text", ...}'); + }); + + it('flags a non-string type (e.g. numeric)', () => { + const bad = doc(para({ type: 123, content: [] } as any)); + const hit = findInvalidNode(bad); + expect(hit).not.toBeNull(); + expect(hit!.path).toBe('node.content[0].content[0]'); + expect(hit!.summary).toContain('missing "type"'); + }); + + it('flags an UNKNOWN node type name that is not in the Docmost schema', () => { + const bad = doc({ + type: 'paragraf', // typo — not a real node + attrs: { id: 'x' }, + content: [text('hi')], + }); + const hit = findInvalidNode(bad); + expect(hit).not.toBeNull(); + expect(hit!.path).toBe('node.content[0]'); + expect(hit!.summary).toContain('unknown node type "paragraf"'); + expect(hit!.summary).toContain('not in the Docmost schema'); + }); + + it('flags an UNKNOWN mark type on an otherwise-valid node', () => { + const bad = doc(para(text('hi', [{ type: 'blink' }]))); + const hit = findInvalidNode(bad); + expect(hit).not.toBeNull(); + expect(hit!.path).toBe('node.content[0].content[0].marks[0]'); + expect(hit!.summary).toContain('unknown mark type "blink"'); + }); + + it('accepts every real Docmost node/mark type it is asked about', () => { + // Known node (callout) and known marks (italic, code) must NOT be flagged. + const good = doc({ + type: 'callout', + attrs: { id: 'c1', type: 'info' }, + content: [para(text('x', [{ type: 'italic' }, { type: 'code' }]))], + }); + expect(findInvalidNode(good)).toBeNull(); + }); + + it('reports the root itself when the root is typeless', () => { + const hit = findInvalidNode({ content: [] } as any); + expect(hit).not.toBeNull(); + expect(hit!.path).toBe('node'); + }); + + it('is null-safe for non-object input', () => { + expect(findInvalidNode(null)).toBeNull(); + expect(findInvalidNode(undefined)).toBeNull(); + expect(findInvalidNode('nope' as any)).toBeNull(); + }); +});