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>
This commit is contained in:
@@ -15,15 +15,19 @@ import { DocmostClient } from "../../build/index.js";
|
|||||||
// the names against the real class — a rename/removal in client.ts would surface
|
// 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.
|
// 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
|
// SCOPE: this guard checks the method-NAME set only, not signatures. It pins the
|
||||||
// directly importable): every method the embedding host depends on MUST exist as
|
// contract from the mcp side (ESM, where the real class is directly importable):
|
||||||
// a function on a real DocmostClient instance. If you rename/remove a client
|
// every method the embedding host depends on MUST exist as a function on a real
|
||||||
// method, this fails here AND you must update DocmostClientLike to match.
|
// 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
|
// Keep the HOST_CONTRACT_METHODS NAME list aligned with the method NAMES declared
|
||||||
// DocmostClientLike interface (the in-app per-user tool adapter only — it is the
|
// in the server's DocmostClientLike interface (the in-app per-user tool adapter
|
||||||
// superset of what either transport calls). Full type-derivation of
|
// only — it is the superset of what either transport calls). Full type-derivation
|
||||||
// DocmostClientLike from this class is deferred (see the staged plan in
|
// of DocmostClientLike from this class is deferred (see the staged plan in
|
||||||
// docmost-client.loader.ts): the package emits no declarations and the real
|
// docmost-client.loader.ts): the package emits no declarations and the real
|
||||||
// (inferred, concrete) return types conflict with the host's loose
|
// (inferred, concrete) return types conflict with the host's loose
|
||||||
// `Record<string,unknown>` + `as`-cast result handling.
|
// `Record<string,unknown>` + `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(", ")}. ` +
|
`DocmostClient is missing host-contract method(s): ${missing.join(", ")}. ` +
|
||||||
`Update packages/mcp/src/client.ts and/or the server's DocmostClientLike ` +
|
`Update packages/mcp/src/client.ts and/or the server's DocmostClientLike ` +
|
||||||
`interface (apps/server/src/core/ai-chat/tools/docmost-client.loader.ts) ` +
|
`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,
|
here,
|
||||||
"../../../../apps/server/src/core/ai-chat/tools/docmost-client.loader.ts",
|
"../../../../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 lines = source.split(/\r?\n/);
|
||||||
|
|
||||||
const startIdx = lines.findIndex((l) =>
|
const startIdx = lines.findIndex((l) =>
|
||||||
@@ -131,13 +148,33 @@ function parseDocmostClientLikeMethods() {
|
|||||||
|
|
||||||
const methods = [];
|
const methods = [];
|
||||||
let closed = false;
|
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++) {
|
for (let i = startIdx + 1; i < lines.length; i++) {
|
||||||
const line = lines[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)) {
|
if (/^\}/.test(line)) {
|
||||||
closed = true;
|
closed = true;
|
||||||
break;
|
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<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]);
|
if (m) methods.push(m[1]);
|
||||||
}
|
}
|
||||||
assert.ok(
|
assert.ok(
|
||||||
|
|||||||
Reference in New Issue
Block a user