Compare commits
4 Commits
feat/184-a
...
refactor/1
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4c7b671950 | ||
|
|
4131deaabb | ||
|
|
5b88e3dddf | ||
|
|
d0ca127d83 |
@@ -5,6 +5,34 @@ import { pathToFileURL } from 'node:url';
|
|||||||
* ESM-only `@docmost/mcp` package. We only need the constructor + the read/write
|
* 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
|
* 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.
|
* `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 = <real DocmostClient type>`
|
||||||
|
* (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<string,unknown>` 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<DocmostClientLike>` 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<DocmostClient, ...>` 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 {
|
export interface DocmostClientLike {
|
||||||
// --- read ---
|
// --- read ---
|
||||||
|
|||||||
212
packages/mcp/test/unit/client-host-contract.test.mjs
Normal file
212
packages/mcp/test/unit/client-host-contract.test.mjs
Normal file
@@ -0,0 +1,212 @@
|
|||||||
|
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";
|
||||||
|
|
||||||
|
// 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.
|
||||||
|
//
|
||||||
|
// 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 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 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
|
||||||
|
// `Record<string,unknown>` + `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 method NAMES stay aligned (this guards names only, ` +
|
||||||
|
`not signatures).`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("HOST_CONTRACT_METHODS has no duplicates", () => {
|
||||||
|
assert.equal(
|
||||||
|
new Set(HOST_CONTRACT_METHODS).size,
|
||||||
|
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",
|
||||||
|
);
|
||||||
|
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) =>
|
||||||
|
/^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;
|
||||||
|
// 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;
|
||||||
|
}
|
||||||
|
// Method-name match: a TS identifier (letters/digits/`_`/`$`, not starting
|
||||||
|
// with a digit) optionally followed by a generic clause (`method<T>(`), 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(
|
||||||
|
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).`,
|
||||||
|
);
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user