From d0ca127d837759f6d557545d9f45a04e35c06eea Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sun, 28 Jun 2026 15:07:43 +0300 Subject: [PATCH 1/4] refactor(ai-chat): drift-guard the DocmostClientLike hand-mirror (#193) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #193's tool-half has two open items. The shared, zod-agnostic tool-spec registry (SHARED_TOOL_SPECS) for the identical tools is already merged (f3fa15e7) and consumed by both layers, so that subset is done. The remaining items are: (a) deriving the layer-3 hand-mirror `DocmostClientLike` from the real client type, and (b) folding more tools into the registry. Both were deferred as risky, and that deferral still holds (verified, see below) — so this change ships the safest concrete increment instead of forcing the risk. What this adds (behaviour-neutral, test-only + a doc comment): - packages/mcp/test/unit/client-host-contract.test.mjs: pins the layer-3 contract from the ESM side, where the real DocmostClient is importable. It asserts every method the in-app `DocmostClientLike` mirror declares exists as a function on a real DocmostClient instance (constructor is side-effect-free). A rename/removal in client.ts now fails this test instead of silently shipping a runtime "x is not a function" into an agent tool call. Negative-case verified (a bogus method name is detected). - docmost-client.loader.ts: replaces the vague mirror comment with a pointer to the guard test and a concrete, empirically-grounded staged plan for the full type-derivation. Verified blockers kept it deferred: @docmost/mcp emits no .d.ts (no `declaration`, no `types` export) and the server has no path mapping for it, so there is no type to import today; and the real methods' inferred CONCRETE return types conflict with the in-app adapter's loose Record + `as`-cast result handling (deriving the exact type breaks the build / forces pervasive double-casts and full-surface test stubs). Out of scope (noted in the issue): the PM<->Markdown converter unification. Verified: server tsc clean; mcp tsc clean; mcp tests 369 pass (367 + 2 new); ai-chat tools specs 51 pass. No behaviour change; committed mcp build untouched (no mcp src changed). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ai-chat/tools/docmost-client.loader.ts | 28 +++++ .../test/unit/client-host-contract.test.mjs | 100 ++++++++++++++++++ 2 files changed, 128 insertions(+) create mode 100644 packages/mcp/test/unit/client-host-contract.test.mjs diff --git a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts index 5b740cfe..e27bfb9a 100644 --- a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts +++ b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts @@ -5,6 +5,34 @@ import { pathToFileURL } from 'node:url'; * ESM-only `@docmost/mcp` package. We only need the constructor + the read/write * methods used by the per-user tool adapter; the full client surface lives in * `packages/mcp/src/client.ts`. Signatures here mirror that file exactly. + * + * DRIFT GUARD: the method NAMES below are runtime-checked against the real + * `DocmostClient` by `packages/mcp/test/unit/client-host-contract.test.mjs` + * (which can import the ESM class directly). If you rename/remove a method here + * or in client.ts, that test fails — so a stale mirror cannot silently ship a + * runtime "x is not a function" into an agent tool call. Keep the two in sync. + * + * STAGED PLAN — full derivation `DocmostClientLike = ` + * (issue #193, layer 3) is intentionally NOT done; it stays a hand-mirror for + * now because of two verified blockers across the ESM(mcp)/CJS(server) boundary: + * 1. `@docmost/mcp` emits NO declaration files (its tsconfig has no + * `declaration`, package.json has no `types`/types-export) and the server + * tsconfig has no path mapping for it — the server only loads it via the + * runtime `import()` trick below, so there is no type to import today. + * 2. The real client methods have inferred, CONCRETE return types; the in-app + * tool adapter reads results through loose `Record` returns + * + `as` casts (e.g. `(result?.data ?? {}) as { title?: string }`). + * Deriving the exact type would make those casts non-overlapping ("may be a + * mistake") and break the build, and `Partial` test stubs + * would have to satisfy the full concrete surface. + * To do it safely later (incrementally): (a) turn on `declaration: true` in + * packages/mcp/tsconfig.json + add a `types` export condition and commit the + * emitted `.d.ts`; (b) `import type { DocmostClient } from '@docmost/mcp'` here + * and replace this interface with a `Pick` of the consumed + * methods; (c) audit every `as` cast in ai-chat-tools.service.ts against the now + * concrete return types (double-cast through `unknown` only where genuinely + * needed); (d) keep the runtime guard test as a belt-and-braces check. Until + * then the guard test above is the cheap, behaviour-neutral protection. */ export interface DocmostClientLike { // --- read --- diff --git a/packages/mcp/test/unit/client-host-contract.test.mjs b/packages/mcp/test/unit/client-host-contract.test.mjs new file mode 100644 index 00000000..c6ca8978 --- /dev/null +++ b/packages/mcp/test/unit/client-host-contract.test.mjs @@ -0,0 +1,100 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { DocmostClient } from "../../build/index.js"; + +// Drift guard for the THIRD hand-written layer of the AI tool set (issue #193, +// layer 3): the in-app server hand-mirrors the DocmostClient method signatures +// it consumes as the `DocmostClientLike` interface in +// apps/server/src/core/ai-chat/tools/docmost-client.loader.ts ("Signatures here +// mirror that file exactly"). That mirror lives across the ESM(mcp)/CJS(server) +// boundary and the package ships NO .d.ts, so the server typecheck cannot verify +// the names against the real class — a rename/removal in client.ts would surface +// only as a runtime "x is not a function" inside an agent tool call. +// +// This test pins the contract from the mcp side (ESM, where the real class is +// directly importable): every method the embedding host depends on MUST exist as +// a function on a real DocmostClient instance. If you rename/remove a client +// method, this fails here AND you must update DocmostClientLike to match. +// +// Keep HOST_CONTRACT_METHODS in sync with the methods declared in the server's +// DocmostClientLike interface (the in-app per-user tool adapter only — it is the +// superset of what either transport calls). Full type-derivation of +// DocmostClientLike from this class is deferred (see the staged plan in +// docmost-client.loader.ts): the package emits no declarations and the real +// (inferred, concrete) return types conflict with the host's loose +// `Record` + `as`-cast result handling. +const HOST_CONTRACT_METHODS = [ + // read + "search", + "getPage", + "getWorkspace", + "getSpaces", + "listPages", + "listSidebarPages", + "getOutline", + "getPageJson", + "getNode", + "getTable", + "listComments", + "getComment", + "checkNewComments", + "listShares", + "listPageHistory", + "getPageHistory", + "diffPageVersions", + "exportPageMarkdown", + // write (page) + "createPage", + "updatePage", + "renamePage", + "movePage", + "deletePage", + "editPageText", + "patchNode", + "insertNode", + "deleteNode", + "updatePageJson", + "tableInsertRow", + "tableDeleteRow", + "tableUpdateCell", + "copyPageContent", + "importPageMarkdown", + "sharePage", + "unsharePage", + "restorePageVersion", + "transformPage", + // write (comment) + "createComment", + "resolveComment", +]; + +test("DocmostClient implements every method the in-app DocmostClientLike mirror declares", () => { + // The constructor is side-effect-free (no network/login on construction): it + // only stores config and creates an axios instance, so it is safe to build a + // throwaway instance here with a dummy token provider. + const client = new DocmostClient({ + apiUrl: "http://127.0.0.1:1/api", + getToken: async () => "test-token", + }); + + const missing = HOST_CONTRACT_METHODS.filter( + (name) => typeof client[name] !== "function", + ); + + assert.deepEqual( + missing, + [], + `DocmostClient is missing host-contract method(s): ${missing.join(", ")}. ` + + `Update packages/mcp/src/client.ts and/or the server's DocmostClientLike ` + + `interface (apps/server/src/core/ai-chat/tools/docmost-client.loader.ts) ` + + `so the hand-mirrored signatures stay in sync.`, + ); +}); + +test("HOST_CONTRACT_METHODS has no duplicates", () => { + assert.equal( + new Set(HOST_CONTRACT_METHODS).size, + HOST_CONTRACT_METHODS.length, + ); +}); -- 2.49.1 From 5b88e3dddf91509bea97026199ba658237ec6bd4 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sun, 28 Jun 2026 23:36:22 +0300 Subject: [PATCH 2/4] test(mcp): drift-guard HOST_CONTRACT_METHODS against DocmostClientLike both ways The contract test only checked one direction (each name in HOST_CONTRACT_METHODS exists on the real DocmostClient). But HOST_CONTRACT_METHODS is itself a hand-copy of the server's DocmostClientLike interface (docmost-client.loader.ts), and that list<->interface link was untested: a method added to the interface + consumed by the adapter but forgotten in the list (or removed from the interface but left in the list) would escape both the server typecheck (the pkg emits no .d.ts) and the existing test (name not in the list) -> a runtime "x is not a function" in a tool call. Parse the method names from the DocmostClientLike interface body (read the .ts source via import.meta.url, scan member-signature lines) and assert.deepEqual them against HOST_CONTRACT_METHODS BOTH ways. Lists are currently identical (39=39), so this is a coverage hole closed, not a live bug. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test/unit/client-host-contract.test.mjs | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/packages/mcp/test/unit/client-host-contract.test.mjs b/packages/mcp/test/unit/client-host-contract.test.mjs index c6ca8978..981ff1a6 100644 --- a/packages/mcp/test/unit/client-host-contract.test.mjs +++ b/packages/mcp/test/unit/client-host-contract.test.mjs @@ -1,5 +1,8 @@ import { test } from "node:test"; import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, resolve } from "node:path"; import { DocmostClient } from "../../build/index.js"; @@ -98,3 +101,72 @@ test("HOST_CONTRACT_METHODS has no duplicates", () => { HOST_CONTRACT_METHODS.length, ); }); + +// Parse the method names declared in the server's `DocmostClientLike` interface +// body. We read the .ts source as plain text (no TS compiler dep, and the file +// lives in the CJS server tree across the ESM boundary): scan from the +// `export interface DocmostClientLike {` line to its closing brace at column 0, +// matching member-signature lines like ` methodName(`. Nested param-object +// braces (`opts: { ... }`) are indented, so only the interface's own closing +// `}` (column 0) ends the scan. +function parseDocmostClientLikeMethods() { + const here = dirname(fileURLToPath(import.meta.url)); + // packages/mcp/test/unit -> repo root is four levels up. + const loaderPath = resolve( + here, + "../../../../apps/server/src/core/ai-chat/tools/docmost-client.loader.ts", + ); + const source = readFileSync(loaderPath, "utf8"); + const lines = source.split(/\r?\n/); + + const startIdx = lines.findIndex((l) => + /^export interface DocmostClientLike\s*\{/.test(l), + ); + assert.notEqual( + startIdx, + -1, + `Could not find "export interface DocmostClientLike {" in ${loaderPath}. ` + + `If the interface was renamed/moved, update this drift-guard test.`, + ); + + const methods = []; + let closed = false; + for (let i = startIdx + 1; i < lines.length; i++) { + const line = lines[i]; + if (/^\}/.test(line)) { + closed = true; + break; + } + const m = /^\s*([a-zA-Z]+)\(/.exec(line); + if (m) methods.push(m[1]); + } + assert.ok( + closed, + `Did not find the closing brace of DocmostClientLike in ${loaderPath}.`, + ); + assert.ok( + methods.length > 0, + `Parsed zero methods from DocmostClientLike in ${loaderPath} — the parser ` + + `is likely out of date with the interface formatting.`, + ); + return methods; +} + +// The point of the guard is to protect the DocmostClientLike mirror <-> client.ts +// link, but HOST_CONTRACT_METHODS is itself a HAND-COPY of that interface kept in +// sync manually. The list<->interface link must be tested too: a method consumed +// by the adapter and added to DocmostClientLike but forgotten here (or removed +// from the interface but left here) would otherwise escape both the server +// typecheck (pkg emits no .d.ts) and the first test above (name not in the list). +// Assert the two agree BOTH ways. +test("HOST_CONTRACT_METHODS exactly mirrors the server's DocmostClientLike interface", () => { + const interfaceMethods = parseDocmostClientLikeMethods(); + assert.deepEqual( + [...HOST_CONTRACT_METHODS].sort(), + [...interfaceMethods].sort(), + `HOST_CONTRACT_METHODS has drifted from the DocmostClientLike interface in ` + + `apps/server/src/core/ai-chat/tools/docmost-client.loader.ts. Add/remove ` + + `method names in HOST_CONTRACT_METHODS so it lists EXACTLY the methods ` + + `declared in that interface (both directions are checked).`, + ); +}); -- 2.49.1 From 4131deaabb863c509fb7fb60c21b6b08867b504e Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sun, 28 Jun 2026 23:54:04 +0300 Subject: [PATCH 3/4] test(mcp): robustify the client-host contract drift-guard parser Architect-review hardening of the bidirectional DocmostClientLike <-> HOST_CONTRACT_METHODS guard (test-only, no production change): - Interface method-name regex now accepts full TS identifiers (digits/_/$) and generic signatures (method(), avoiding a future benign false-FAIL. - Skip /* ... */ block comments in the interface body so a `name(` line inside one is not falsely parsed as a method. - Wrap the cross-package readFileSync with a clear "expected monorepo layout" error instead of a bare ENOENT when run outside the monorepo. - Narrow the guard's comments/error to state plainly it checks the method-NAME set only; signature parity remains the deferred staged-plan item. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test/unit/client-host-contract.test.mjs | 59 +++++++++++++++---- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/packages/mcp/test/unit/client-host-contract.test.mjs b/packages/mcp/test/unit/client-host-contract.test.mjs index 981ff1a6..f807b082 100644 --- a/packages/mcp/test/unit/client-host-contract.test.mjs +++ b/packages/mcp/test/unit/client-host-contract.test.mjs @@ -15,15 +15,19 @@ import { DocmostClient } from "../../build/index.js"; // the names against the real class — a rename/removal in client.ts would surface // only as a runtime "x is not a function" inside an agent tool call. // -// This test pins the contract from the mcp side (ESM, where the real class is -// directly importable): every method the embedding host depends on MUST exist as -// a function on a real DocmostClient instance. If you rename/remove a client -// method, this fails here AND you must update DocmostClientLike to match. +// SCOPE: this guard checks the method-NAME set only, not signatures. It pins the +// contract from the mcp side (ESM, where the real class is directly importable): +// every method the embedding host depends on MUST exist as a function on a real +// DocmostClient instance. If you rename/remove a client method, this fails here +// AND you must update DocmostClientLike to match. It does NOT verify parameter or +// return-type parity — signature drift between the hand-mirror and client.ts can +// still ship silently; full signature/type parity is the deferred staged-plan +// item below. // -// Keep HOST_CONTRACT_METHODS in sync with the methods declared in the server's -// DocmostClientLike interface (the in-app per-user tool adapter only — it is the -// superset of what either transport calls). Full type-derivation of -// DocmostClientLike from this class is deferred (see the staged plan in +// Keep the HOST_CONTRACT_METHODS NAME list aligned with the method NAMES declared +// in the server's DocmostClientLike interface (the in-app per-user tool adapter +// only — it is the superset of what either transport calls). Full type-derivation +// of DocmostClientLike from this class is deferred (see the staged plan in // docmost-client.loader.ts): the package emits no declarations and the real // (inferred, concrete) return types conflict with the host's loose // `Record` + `as`-cast result handling. @@ -91,7 +95,8 @@ test("DocmostClient implements every method the in-app DocmostClientLike mirror `DocmostClient is missing host-contract method(s): ${missing.join(", ")}. ` + `Update packages/mcp/src/client.ts and/or the server's DocmostClientLike ` + `interface (apps/server/src/core/ai-chat/tools/docmost-client.loader.ts) ` + - `so the hand-mirrored signatures stay in sync.`, + `so the hand-mirrored method NAMES stay aligned (this guards names only, ` + + `not signatures).`, ); }); @@ -116,7 +121,19 @@ function parseDocmostClientLikeMethods() { here, "../../../../apps/server/src/core/ai-chat/tools/docmost-client.loader.ts", ); - const source = readFileSync(loaderPath, "utf8"); + let source; + try { + source = readFileSync(loaderPath, "utf8"); + } catch (err) { + if (err && err.code === "ENOENT") { + throw new Error( + `Expected monorepo layout; server tree at ${loaderPath} not found. ` + + `This drift-guard reads the server's DocmostClientLike interface via a ` + + `fixed relative path and must run from inside the monorepo checkout.`, + ); + } + throw err; + } const lines = source.split(/\r?\n/); const startIdx = lines.findIndex((l) => @@ -131,13 +148,33 @@ function parseDocmostClientLikeMethods() { const methods = []; let closed = false; + // Track whether we are inside a `/* ... */` block comment. Inner lines of a + // block comment need NOT start with `*`, so a `name(` line inside one would be + // falsely parsed as an interface method without this. (`//` line comments can + // never match the method regex below since they start with `/`.) + let inBlockComment = false; for (let i = startIdx + 1; i < lines.length; i++) { const line = lines[i]; + if (inBlockComment) { + // Stay in the block until we see its closing `*/`. + if (line.includes("*/")) inBlockComment = false; + continue; + } + // Enter a block comment only when it opens without closing on the same line; + // a self-contained `/* ... */` on one line cannot precede a method name we + // care about (such lines start with `/`, so the method regex won't match). + if (line.includes("/*") && !line.includes("*/")) { + inBlockComment = true; + continue; + } if (/^\}/.test(line)) { closed = true; break; } - const m = /^\s*([a-zA-Z]+)\(/.exec(line); + // Method-name match: a TS identifier (letters/digits/`_`/`$`, not starting + // with a digit) optionally followed by a generic clause (`method(`), then + // the opening paren of the signature. + const m = /^\s*([A-Za-z_$][A-Za-z0-9_$]*)\s*(?:<[^>]*>)?\(/.exec(line); if (m) methods.push(m[1]); } assert.ok( -- 2.49.1 From 4c7b67195034a502da7a28bb4bc079a2fadf240a Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Mon, 29 Jun 2026 01:59:10 +0300 Subject: [PATCH 4/4] =?UTF-8?q?docs(#193):=20correct=20contract-guard=20co?= =?UTF-8?q?mment=20=E2=80=94=20interface=20is=20a=20subset,=20not=20supers?= =?UTF-8?q?et?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DocmostClientLike mirror covers only methods the in-app adapter consumes; the standalone MCP transport calls additional client methods not tracked here (covered by its own typecheck). Fixes the misleading 'superset' wording (F2). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/mcp/test/unit/client-host-contract.test.mjs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/mcp/test/unit/client-host-contract.test.mjs b/packages/mcp/test/unit/client-host-contract.test.mjs index f807b082..ad0abab7 100644 --- a/packages/mcp/test/unit/client-host-contract.test.mjs +++ b/packages/mcp/test/unit/client-host-contract.test.mjs @@ -26,7 +26,10 @@ import { DocmostClient } from "../../build/index.js"; // // Keep the HOST_CONTRACT_METHODS NAME list aligned with the method NAMES declared // in the server's DocmostClientLike interface (the in-app per-user tool adapter -// only — it is the superset of what either transport calls). Full type-derivation +// only — it is a SUBSET of the DocmostClient surface — covers only what the in-app adapter +// consumes; the standalone MCP transport (packages/mcp/src/index.ts) calls additional +// client methods (insertImage/replaceImage/deleteComment/updateComment/insertFootnote/ +// uploadImage) that this guard does NOT track — the MCP transport's own typecheck covers those). Full type-derivation // of DocmostClientLike from this class is deferred (see the staged plan in // docmost-client.loader.ts): the package emits no declarations and the real // (inferred, concrete) return types conflict with the host's loose -- 2.49.1