Compare commits

...

5 Commits

Author SHA1 Message Date
claude code agent 227
888deba891 docs(#193): drop uploadImage from MCP-transport method list in contract-guard comment (F3)
uploadImage is internal to client.ts (called by insertImage/replaceImage);
the MCP transport (index.ts) does not call it directly. Remove it from the
comment's list of transport-called methods.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-29 14:07:02 +03:00
claude code agent 227
4c7b671950 docs(#193): correct contract-guard comment — interface is a subset, not superset
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) <noreply@anthropic.com>
2026-06-29 01:59:10 +03:00
claude code agent 227
4131deaabb 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<T>(), 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) <noreply@anthropic.com>
2026-06-28 23:54:04 +03:00
claude code agent 227
5b88e3dddf 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) <noreply@anthropic.com>
2026-06-28 23:36:22 +03:00
claude code agent 227
d0ca127d83 refactor(ai-chat): drift-guard the DocmostClientLike hand-mirror (#193)
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<string,unknown> + `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) <noreply@anthropic.com>
2026-06-28 15:07:43 +03:00
2 changed files with 240 additions and 0 deletions

View File

@@ -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 = <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 {
// --- read ---

View 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)
// 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).`,
);
});