Merge pull request '#114 refactor(ai-chat): shared parseNodeArg helper; keep duplication backlog doc' (#114) from refactor/ai-chat-tool-spec-registry into develop

# Conflicts:
#	apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts

apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts

@@ -13,6 +13,7 @@ import {
   type DocmostClientLike,
 } from './docmost-client.loader';
 import { resolveCurrentPageResult } from './current-page.util';
+import { parseNodeArg } from './parse-node-arg';
 
 /**
  * Per-user, per-request adapter that exposes Docmost READ operations to the
@@ -705,14 +706,7 @@ export class AiChatToolsService {
           // Parity with the standalone MCP server (index.ts patch_node): the
           // model sometimes serializes the node as a JSON string. Parse it
           // before the client's typeof-object guard rejects it.
-          let parsedNode = node;
+          const parsedNode = parseNodeArg(node);
-          if (typeof node === 'string') {
-            try {
-              parsedNode = JSON.parse(node);
-            } catch {
-              throw new Error('node was a string but not valid JSON');
-            }
-          }
           return await client.patchNode(pageId, nodeId, parsedNode);
         },
       }),
@@ -764,14 +758,7 @@ export class AiChatToolsService {
           // Parity with the standalone MCP server (index.ts insert_node): the
           // model sometimes serializes the node as a JSON string. Parse it
           // before the client's typeof-object guard rejects it.
-          let parsedNode = node;
+          const parsedNode = parseNodeArg(node);
-          if (typeof node === 'string') {
-            try {
-              parsedNode = JSON.parse(node);
-            } catch {
-              throw new Error('node was a string but not valid JSON');
-            }
-          }
           return await client.insertNode(pageId, parsedNode, {
             position,
             anchorNodeId,
@@ -820,14 +807,9 @@ export class AiChatToolsService {
           let doc;
           if (content === undefined || content === null) {
             doc = undefined;
-          } else if (typeof content === 'string') {
-            try {
-              doc = JSON.parse(content);
-            } catch {
-              throw new Error('content was a string but not valid JSON');
-            }
           } else {
-            doc = content;
+            // String -> JSON.parse (throwing on invalid); object passes through.
+            doc = parseNodeArg(content, 'content was a string but not valid JSON');
           }
           return await client.updatePageJson(pageId, doc, title);
         },

apps/server/src/core/ai-chat/tools/parse-node-arg.spec.ts

@@ -0,0 +1,37 @@
+import { parseNodeArg } from './parse-node-arg';
+
+/**
+ * Unit tests for the in-app `parseNodeArg` helper. It mirrors the standalone
+ * MCP helper (packages/mcp/src/lib/parse-node-arg.ts) and is used by the
+ * patchNode / insertNode / updatePageJson tool adapters. Behavior must be
+ * byte-identical: object passthrough, valid-string parse, invalid-string throw.
+ */
+describe('parseNodeArg', () => {
+  it('passes an object through unchanged', () => {
+    const obj = { type: 'paragraph', content: [] };
+    expect(parseNodeArg(obj)).toBe(obj);
+  });
+
+  it('passes undefined/null through unchanged', () => {
+    expect(parseNodeArg(undefined)).toBeUndefined();
+    expect(parseNodeArg(null)).toBeNull();
+  });
+
+  it('parses a valid JSON string into an object', () => {
+    expect(parseNodeArg('{"type":"paragraph"}')).toEqual({
+      type: 'paragraph',
+    });
+  });
+
+  it('throws the default message on an invalid JSON string', () => {
+    expect(() => parseNodeArg('{not json')).toThrow(
+      'node was a string but not valid JSON',
+    );
+  });
+
+  it('throws a custom message on an invalid JSON string', () => {
+    expect(() =>
+      parseNodeArg('{not json', 'content was a string but not valid JSON'),
+    ).toThrow('content was a string but not valid JSON');
+  });
+});

apps/server/src/core/ai-chat/tools/parse-node-arg.ts

@@ -0,0 +1,26 @@
+// The model sometimes serializes a ProseMirror node arg as a JSON string
+// instead of an object. Normalize: parse a string to an object (throwing on
+// invalid JSON), pass an object through unchanged. Shared by patchNode /
+// insertNode (and the analogous updatePageJson content parsing).
+//
+// This is behaviorally identical to `packages/mcp/src/lib/parse-node-arg.ts`
+// (the function logic, default/explicit throw messages and branch order match;
+// only comments and quote style differ). We cannot import that helper here:
+// `@docmost/mcp` is ESM-only and this server
+// compiles with module:commonjs, so it is loaded at runtime via the
+// `new Function('import()')` trick (see docmost-client.loader.ts). Sharing
+// runtime code across that ESM/CJS boundary by a normal import is impossible,
+// hence the mirrored copy.
+export function parseNodeArg(
+  node: unknown,
+  errMsg = 'node was a string but not valid JSON',
+): unknown {
+  if (typeof node === 'string') {
+    try {
+      return JSON.parse(node);
+    } catch {
+      throw new Error(errMsg);
+    }
+  }
+  return node;
+}

docs/backlog/ai-chat-tool-definitions-duplicated.md

@@ -1,8 +1,26 @@
 # Дублирование определений инструментов: in-app агент vs standalone MCP-пакет
 
-Статус: **зафиксировано в беклоге, код не менялся.** Это forward-looking
+Статус: **частично закрыто.** Квирк «node как объект ИЛИ JSON-строка» вынесен
-стоимость поддержки, НЕ баг — код корректен сегодня. Фиксируем, чтобы при
+в общий хелпер `parseNodeArg` (см. «Прогресс» ниже); остальной долг (единый
-росте набора инструментов (см. §16) долг не разъезжался молча.
+реестр спеков + унификация конвертера) всё ещё открыт. Это forward-looking
+стоимость поддержки, НЕ баг — код корректен сегодня. Держим запись открытой,
+чтобы при росте набора инструментов долг не разъезжался молча.
+
+## Прогресс
+
+- ✅ **Квирк node-arg вынесен в хелпер** (`refactor/ai-chat-tool-spec-registry`,
+  PR #114). Шесть рукописных копий нормализации «node как объект ИЛИ
+  JSON-строка» свёрнуты в `parseNodeArg`: по одному источнику на пакет —
+  `packages/mcp/src/lib/parse-node-arg.ts` (standalone) и
+  `apps/server/src/core/ai-chat/tools/parse-node-arg.ts` (in-app). Две копии
+  намеренны (ESM/CJS-граница), поведение тождественно.
+- ⏳ **Единый реестр спеков** (схема + описание на инструмент) и **вывод
+  `DocmostClientLike` из реального типа** — отложены (см. «Фикс»): требуют
+  пересечения ESM/CJS-границы для данных+zod и ломают тест-стабы in-app
+  инструментов при точных типах. Делать инкрементально.
+- ⏳ **Унификация конвертера ProseMirror ↔ Markdown** — открыта (см. раздел
+  «Расширение …» ниже); на неё опирается план git-синка
+  (`docs/git-sync-plan.md`).
 
 ## Суть
 
@@ -28,12 +46,13 @@ parity-баги (расхождение копий) приходится чин
 ## Что именно продублировано (с подтверждением по коду)
 
 - **zod-схема + описание** каждого инструмента — в слоях 1 и 2 целиком.
-- **Квирк «node как объект ИЛИ JSON-строка»** реализован дважды (НЕ в общем
+- ~~**Квирк «node как объект ИЛИ JSON-строка»** реализован дважды (НЕ в общем
-  клиенте):
+  клиенте)~~ — **закрыто (PR #114):** вынесен в `parseNodeArg` (по хелперу на
-  - in-app: `ai-chat-tools.service.ts:686` (patchNode), `:745` (insertNode),
+  пакет), 6 inline-копий устранены:
-    `:800` (updatePageJson);
+  - in-app: `patchNode`, `insertNode`, `updatePageJson` →
-  - standalone: `index.ts:526` (patch_node), `:578` (insert_node), `:350`
+    `apps/server/src/core/ai-chat/tools/parse-node-arg.ts`;
-    (update_page_json).
+  - standalone: `patch_node`, `insert_node`, `update_page_json` →
+    `packages/mcp/src/lib/parse-node-arg.ts`.
 - **Guardrail/семантика `transformPage` (dryRun)** описана в обоих:
   `ai-chat-tools.service.ts:~935` и `index.ts:~1006`.
 

packages/mcp/build/index.js

@@ -4,6 +4,7 @@ import { readFileSync } from "fs";
 import { fileURLToPath } from "url";
 import { dirname, join } from "path";
 import { DocmostClient } from "./client.js";
+import { parseNodeArg } from "./lib/parse-node-arg.js";
 // Re-export the client and its config type so embedding hosts (e.g. the gitmost
 // NestJS server) can `import('@docmost/mcp')` and construct a DocmostClient
 // directly — for the credentials variant OR the per-user getToken variant.
@@ -245,16 +246,9 @@ export function createDocmostMcpServer(config) {
         if (content === undefined || content === null) {
             doc = undefined;
         }
-        else if (typeof content === "string") {
-            try {
-                doc = JSON.parse(content);
-            }
-            catch {
-                throw new Error("content was a string but not valid JSON");
-            }
-        }
         else {
-            doc = content;
+            // String -> JSON.parse (throwing on invalid); object passes through.
+            doc = parseNodeArg(content, "content was a string but not valid JSON");
         }
         const result = await docmostClient.updatePageJson(pageId, doc, title);
         return jsonContent(result);
@@ -379,18 +373,7 @@ export function createDocmostMcpServer(config) {
                 "JSON object or JSON string both accepted."),
         },
     }, async ({ pageId, nodeId, node }) => {
-        let parsedNode;
+        const parsedNode = parseNodeArg(node);
-        if (typeof node === "string") {
-            try {
-                parsedNode = JSON.parse(node);
-            }
-            catch {
-                throw new Error("node was a string but not valid JSON");
-            }
-        }
-        else {
-            parsedNode = node;
-        }
         const result = await docmostClient.patchNode(pageId, nodeId, parsedNode);
         return jsonContent(result);
     });
@@ -425,18 +408,7 @@ export function createDocmostMcpServer(config) {
             anchorText: z.string().optional(),
         },
     }, async ({ pageId, node, position, anchorNodeId, anchorText }) => {
-        let parsedNode;
+        const parsedNode = parseNodeArg(node);
-        if (typeof node === "string") {
-            try {
-                parsedNode = JSON.parse(node);
-            }
-            catch {
-                throw new Error("node was a string but not valid JSON");
-            }
-        }
-        else {
-            parsedNode = node;
-        }
         const result = await docmostClient.insertNode(pageId, parsedNode, {
             position,
             anchorNodeId,

packages/mcp/build/lib/parse-node-arg.js

@@ -0,0 +1,15 @@
+// The model sometimes serializes a ProseMirror node arg as a JSON string
+// instead of an object. Normalize: parse a string to an object (throwing on
+// invalid JSON), pass an object through unchanged. Shared by patch_node /
+// insert_node (and the analogous update_page_json content parsing).
+export function parseNodeArg(node, errMsg = "node was a string but not valid JSON") {
+    if (typeof node === "string") {
+        try {
+            return JSON.parse(node);
+        }
+        catch {
+            throw new Error(errMsg);
+        }
+    }
+    return node;
+}

packages/mcp/src/index.ts

@@ -4,6 +4,7 @@ import { readFileSync } from "fs";
 import { fileURLToPath } from "url";
 import { dirname, join } from "path";
 import { DocmostClient, DocmostMcpConfig } from "./client.js";
+import { parseNodeArg } from "./lib/parse-node-arg.js";
 
 // Re-export the client and its config type so embedding hosts (e.g. the gitmost
 // NestJS server) can `import('@docmost/mcp')` and construct a DocmostClient
@@ -354,14 +355,9 @@ server.registerTool(
     let doc;
     if (content === undefined || content === null) {
       doc = undefined;
-    } else if (typeof content === "string") {
-      try {
-        doc = JSON.parse(content);
-      } catch {
-        throw new Error("content was a string but not valid JSON");
-      }
     } else {
-      doc = content;
+      // String -> JSON.parse (throwing on invalid); object passes through.
+      doc = parseNodeArg(content, "content was a string but not valid JSON");
     }
     const result = await docmostClient.updatePageJson(pageId, doc, title);
     return jsonContent(result);
@@ -529,16 +525,7 @@ server.registerTool(
     },
   },
   async ({ pageId, nodeId, node }) => {
-    let parsedNode;
+    const parsedNode = parseNodeArg(node);
-    if (typeof node === "string") {
-      try {
-        parsedNode = JSON.parse(node);
-      } catch {
-        throw new Error("node was a string but not valid JSON");
-      }
-    } else {
-      parsedNode = node;
-    }
     const result = await docmostClient.patchNode(pageId, nodeId, parsedNode);
     return jsonContent(result);
   },
@@ -581,16 +568,7 @@ server.registerTool(
     },
   },
   async ({ pageId, node, position, anchorNodeId, anchorText }) => {
-    let parsedNode;
+    const parsedNode = parseNodeArg(node);
-    if (typeof node === "string") {
-      try {
-        parsedNode = JSON.parse(node);
-      } catch {
-        throw new Error("node was a string but not valid JSON");
-      }
-    } else {
-      parsedNode = node;
-    }
     const result = await docmostClient.insertNode(pageId, parsedNode, {
       position,
       anchorNodeId,

packages/mcp/src/lib/parse-node-arg.ts

@@ -0,0 +1,17 @@
+// The model sometimes serializes a ProseMirror node arg as a JSON string
+// instead of an object. Normalize: parse a string to an object (throwing on
+// invalid JSON), pass an object through unchanged. Shared by patch_node /
+// insert_node (and the analogous update_page_json content parsing).
+export function parseNodeArg(
+  node: unknown,
+  errMsg = "node was a string but not valid JSON",
+): unknown {
+  if (typeof node === "string") {
+    try {
+      return JSON.parse(node);
+    } catch {
+      throw new Error(errMsg);
+    }
+  }
+  return node;
+}

packages/mcp/test/unit/parse-node-arg.test.mjs

@@ -0,0 +1,32 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import { parseNodeArg } from "../../build/lib/parse-node-arg.js";
+
+test("parseNodeArg passes an object through unchanged", () => {
+  const obj = { type: "paragraph", content: [] };
+  assert.strictEqual(parseNodeArg(obj), obj);
+});
+
+test("parseNodeArg passes undefined/null through unchanged", () => {
+  assert.strictEqual(parseNodeArg(undefined), undefined);
+  assert.strictEqual(parseNodeArg(null), null);
+});
+
+test("parseNodeArg parses a valid JSON string", () => {
+  const parsed = parseNodeArg('{"type":"paragraph"}');
+  assert.deepStrictEqual(parsed, { type: "paragraph" });
+});
+
+test("parseNodeArg throws the default message on invalid JSON string", () => {
+  assert.throws(() => parseNodeArg("{not json"), {
+    message: "node was a string but not valid JSON",
+  });
+});
+
+test("parseNodeArg throws a custom message on invalid JSON string", () => {
+  assert.throws(
+    () => parseNodeArg("{not json", "content was a string but not valid JSON"),
+    { message: "content was a string but not valid JSON" },
+  );
+});