refactor(mcp): вывести DocmostClientLike/SharedToolSpec из реального типа клиента — убить ручные зеркала (#446)
Восстановленный отложенный долг #294: @docmost/mcp не отдавал .d.ts, поэтому в сервере жили ТРИ дрейфующие ручные копии одних и тех же имён/сигнатур (DocmostClientLike ~230 строк, копия SharedToolSpec, name-only HOST_CONTRACT_ METHODS-тест). In-app execute-тела зовут клиент ПОЗИЦИОННО, так что перестановка параметра в client.ts доезжала до прода рантайм-ошибкой без сигнала на компиляции. - declaration:true (+declarationMap) в packages/mcp/tsconfig.json; types-экспорт в package.json (exports → conditional {types, default} для . и ./http; require.resolve/dynamic-import резолвят default → build/index.js, рантайм не тронут). build/index.d.ts эмитится, реэкспортит DocmostClient + SharedToolSpec. Правок исходников пакета для эмита НЕ потребовалось. - DocmostClientLike → Pick<DocmostClient, 48 методов> из type-only import (стёрт на компиляции, ESM/CJS-границу не задевает); ручное зеркало удалено. - SharedToolSpec → type-only реэкспорт из пакета; ручная копия удалена. - client-host-contract.test.mjs удалён целиком — имена И сигнатуры теперь проверяет tsc. - Позиционная безопасность: never-called __assertClientCallContract(client: DocmostClientLike) воспроизводит каждый позиционный вызов с типизированными плейсхолдерами (AI-SDK стирает вход execute-замыканий в any, иначе позиционные вызовы не проверялись). Перестановка параметров client.ts → ошибка компиляции сервера ровно тут. Loose as-касты в ai-chat-tools.service не потребовали правок; as any не добавлялся. Внутреннее ревью: APPROVE. Runtime resolution через conditional exports не сломан (разобрано для прод-инсталляции, не только symlink); покрытие __assertClientCallContract полное (48 call-sites == union == assert, сверено программно); Pick полон; демонстрация reorder → TS2345 в assert. Единственная находка (Promise<any> в части возвратов) предсуществующая в client.ts, вне цели PR. Стоит на #447 (закрытие skew build/vs/src) — мержить после него. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,17 @@
|
||||
import { AiChatToolsService } from './ai-chat-tools.service';
|
||||
import * as loader from './docmost-client.loader';
|
||||
import type { DocmostClientLike } from './docmost-client.loader';
|
||||
|
||||
// Test-double type for the loopback client. `DocmostClientLike` is now DERIVED
|
||||
// from the real `DocmostClient` (issue #446), so its method RETURN types are the
|
||||
// concrete client shapes. These stubs deliberately return minimal recording
|
||||
// shapes (e.g. `{ ok: true }`), which no longer satisfy those concrete returns —
|
||||
// so the doubles are typed with the same method NAMES but loose async returns.
|
||||
// Each is still cast to `DocmostClientLike` at the (return-erased) mock site, so
|
||||
// the positional-call type-safety on the PRODUCTION client is unaffected.
|
||||
type FakeDocmostClient = Partial<
|
||||
Record<keyof DocmostClientLike, (...args: any[]) => Promise<any>>
|
||||
>;
|
||||
// The real zod-agnostic shared tool-spec registry. It has no runtime deps, so
|
||||
// importing the TS source directly keeps these mocks honest: the service builds
|
||||
// the shared tools from exactly the specs the package ships, not a hand-stub.
|
||||
@@ -31,7 +42,7 @@ describe('AiChatToolsService deletePage guardrail (H4)', () => {
|
||||
|
||||
// Minimal fake DocmostClient: only the write methods the tools touch need to
|
||||
// exist; deletePage records its args. No network, no ESM import.
|
||||
const fakeClient: Partial<DocmostClientLike> = {
|
||||
const fakeClient: FakeDocmostClient = {
|
||||
deletePage: (...args: unknown[]) => {
|
||||
deletePageCalls.push(args);
|
||||
return Promise.resolve({ success: true });
|
||||
@@ -160,7 +171,7 @@ describe('AiChatToolsService deletePage guardrail (H4)', () => {
|
||||
describe('AiChatToolsService expanded toolset guardrails', () => {
|
||||
// No client method is invoked here — every assertion is on tool presence /
|
||||
// input schema — so an empty fake client is sufficient.
|
||||
const fakeClient: Partial<DocmostClientLike> = {};
|
||||
const fakeClient: FakeDocmostClient = {};
|
||||
|
||||
const tokenServiceStub = {
|
||||
generateAccessToken: jest.fn().mockResolvedValue('access-token'),
|
||||
@@ -265,7 +276,7 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
|
||||
const insertNodeCalls: unknown[][] = [];
|
||||
const updatePageJsonCalls: unknown[][] = [];
|
||||
|
||||
const fakeClient: Partial<DocmostClientLike> = {
|
||||
const fakeClient: FakeDocmostClient = {
|
||||
patchNode: (...args: unknown[]) => {
|
||||
patchNodeCalls.push(args);
|
||||
return Promise.resolve({ ok: true });
|
||||
@@ -439,7 +450,7 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
|
||||
* getOutline) are exercised here end-to-end through forUser().
|
||||
*/
|
||||
describe('AiChatToolsService model-friendly input validation (#190)', () => {
|
||||
const fakeClient: Partial<DocmostClientLike> = {};
|
||||
const fakeClient: FakeDocmostClient = {};
|
||||
const tokenServiceStub = {
|
||||
generateAccessToken: jest.fn().mockResolvedValue('access-token'),
|
||||
generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
|
||||
@@ -557,7 +568,7 @@ describe('AiChatToolsService #294 changed execute wirings', () => {
|
||||
tableDeleteRow: [],
|
||||
tableUpdateCell: [],
|
||||
};
|
||||
const fakeClient: Partial<DocmostClientLike> = {
|
||||
const fakeClient: FakeDocmostClient = {
|
||||
movePage: (...args: unknown[]) => {
|
||||
calls.movePage.push(args);
|
||||
return Promise.resolve({ success: true });
|
||||
@@ -666,7 +677,7 @@ describe('AiChatToolsService #410 footnote + image tools', () => {
|
||||
insertImage: [],
|
||||
replaceImage: [],
|
||||
};
|
||||
const fakeClient: Partial<DocmostClientLike> = {
|
||||
const fakeClient: FakeDocmostClient = {
|
||||
insertFootnote: (...args: unknown[]) => {
|
||||
calls.insertFootnote.push(args);
|
||||
return Promise.resolve({ success: true, footnoteId: 'fn1', reused: false });
|
||||
|
||||
@@ -26,6 +26,100 @@ import {
|
||||
type ToolCatalogEntry,
|
||||
} from './tool-tiers';
|
||||
|
||||
/**
|
||||
* Compile-time contract (issue #446): the in-app tool `execute` closures below
|
||||
* call the loopback `DocmostClient` POSITIONALLY (e.g.
|
||||
* `client.drawioGet(pageId, node, format ?? 'xml')`). Those closures receive an
|
||||
* AI-SDK-erased (`any`) input, so a positional call inside them is NOT checked
|
||||
* against the real signature — a parameter reorder/type-change in
|
||||
* `packages/mcp/src/client.ts` would otherwise reach production as a runtime
|
||||
* "wrong argument" tool failure with zero compile signal (the restored #294
|
||||
* debt). This never-called function reproduces every positional call with
|
||||
* correctly-typed placeholder arguments against the DERIVED `DocmostClientLike`
|
||||
* (a `Pick` of the real `DocmostClient`), so any such reorder/rename becomes a
|
||||
* SERVER COMPILE ERROR here. It emits nothing (types only) and is never invoked;
|
||||
* keep each call in lockstep with the matching `execute` body below.
|
||||
*/
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
function __assertClientCallContract(client: DocmostClientLike): void {
|
||||
// Placeholders standing in for the AI-SDK-erased execute inputs. Their types
|
||||
// are deliberately concrete so the positional calls are checked end-to-end.
|
||||
const s = '' as string;
|
||||
const n = 0 as number;
|
||||
const node: unknown = null;
|
||||
const edits: Array<{ find: string; replace: string; replaceAll?: boolean }> =
|
||||
[];
|
||||
const cells: string[] = [];
|
||||
const align = undefined as 'left' | 'center' | 'right' | undefined;
|
||||
|
||||
// --- read ---
|
||||
void client.search(s, undefined, n);
|
||||
void client.getPage(s);
|
||||
void client.getPageRaw(s);
|
||||
void client.getWorkspace();
|
||||
void client.getSpaces();
|
||||
void client.listPages(s, n, true);
|
||||
void client.listSidebarPages(s, s);
|
||||
void client.getOutline(s);
|
||||
void client.getPageJson(s);
|
||||
void client.getNode(s, s);
|
||||
void client.searchInPage(s, s, {
|
||||
regex: true,
|
||||
caseSensitive: true,
|
||||
limit: n,
|
||||
});
|
||||
void client.getTable(s, s);
|
||||
void client.listComments(s, true);
|
||||
void client.getComment(s);
|
||||
void client.checkNewComments(s, s, s);
|
||||
void client.listShares();
|
||||
void client.listPageHistory(s, s);
|
||||
void client.getPageHistory(s);
|
||||
void client.diffPageVersions(s, s, s);
|
||||
void client.exportPageMarkdown(s);
|
||||
// --- write (page) ---
|
||||
void client.createPage(s, s, s, s);
|
||||
void client.updatePage(s, s, s);
|
||||
void client.renamePage(s, s);
|
||||
void client.movePage(s, s, s);
|
||||
void client.deletePage(s);
|
||||
void client.editPageText(s, edits);
|
||||
void client.patchNode(s, s, node);
|
||||
void client.insertNode(s, node, {
|
||||
position: 'append',
|
||||
anchorNodeId: s,
|
||||
anchorText: s,
|
||||
});
|
||||
void client.deleteNode(s, s);
|
||||
void client.updatePageJson(s, node, s);
|
||||
void client.tableInsertRow(s, s, cells, n);
|
||||
void client.tableDeleteRow(s, s, n);
|
||||
void client.tableUpdateCell(s, s, n, n, s);
|
||||
void client.copyPageContent(s, s);
|
||||
void client.importPageMarkdown(s, s);
|
||||
void client.sharePage(s, true);
|
||||
void client.unsharePage(s);
|
||||
void client.restorePageVersion(s);
|
||||
void client.transformPage(s, s, { dryRun: true });
|
||||
void client.stashPage(s);
|
||||
// --- write (image / footnote), in-app since #410 ---
|
||||
void client.insertFootnote(s, s, s);
|
||||
void client.insertImage(s, s, {
|
||||
align,
|
||||
alt: s,
|
||||
replaceText: s,
|
||||
afterText: s,
|
||||
});
|
||||
void client.replaceImage(s, s, s, { align, alt: s });
|
||||
// --- draw.io diagrams (#423) ---
|
||||
void client.drawioGet(s, s, 'xml');
|
||||
void client.drawioCreate(s, { position: 'append', anchorNodeId: s }, s, s);
|
||||
void client.drawioUpdate(s, s, s, s);
|
||||
// --- write (comment) ---
|
||||
void client.createComment(s, s, 'inline', s, s, s);
|
||||
void client.resolveComment(s, true);
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-user, per-request adapter that exposes Docmost READ operations to the
|
||||
* agent as AI SDK tools (STAGE A = read only).
|
||||
|
||||
@@ -7,6 +7,16 @@ import type {
|
||||
DocmostClientLike,
|
||||
CommentSignalTrackerLike,
|
||||
} from './docmost-client.loader';
|
||||
|
||||
// Test-double type for the loopback client. `DocmostClientLike` is now DERIVED
|
||||
// from the real `DocmostClient` (issue #446), so its method RETURN types are the
|
||||
// concrete client shapes. These probe stubs deliberately return minimal shapes
|
||||
// (e.g. `getPageRaw` yielding only `{ title }`), so the doubles use the same
|
||||
// method NAMES but loose async returns; each is cast to `DocmostClientLike` at
|
||||
// the (return-erased) mock site, leaving production positional-call safety intact.
|
||||
type FakeDocmostClient = Partial<
|
||||
Record<keyof DocmostClientLike, (...args: any[]) => Promise<any>>
|
||||
>;
|
||||
import { SHARED_TOOL_SPECS } from '../../../../../../packages/mcp/src/tool-specs';
|
||||
// The REAL shared tracker factory, imported from source (same cross-boundary
|
||||
// approach the tool-specs spec uses) so the in-app wiring is exercised against
|
||||
@@ -268,7 +278,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
|
||||
// seeded at forUser time).
|
||||
const future = new Date(Date.now() + 3_600_000).toISOString();
|
||||
|
||||
function buildService(fakeClient: Partial<DocmostClientLike>) {
|
||||
function buildService(fakeClient: FakeDocmostClient) {
|
||||
jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({
|
||||
DocmostClient: function () {
|
||||
return fakeClient as DocmostClientLike;
|
||||
@@ -317,7 +327,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
|
||||
afterEach(() => jest.restoreAllMocks());
|
||||
|
||||
it('emits the signal (model-only) on a non-comment tool when a new comment exists', async () => {
|
||||
const fakeClient: Partial<DocmostClientLike> = {
|
||||
const fakeClient: FakeDocmostClient = {
|
||||
getPage: async () => ({
|
||||
data: { title: 'Иранские языки', content: 'body' },
|
||||
success: true,
|
||||
@@ -342,7 +352,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
|
||||
});
|
||||
|
||||
it('does NOT add the signal to the listComments tool itself (tautological)', async () => {
|
||||
const fakeClient: Partial<DocmostClientLike> = {
|
||||
const fakeClient: FakeDocmostClient = {
|
||||
listComments: async () => ({
|
||||
items: [{ createdAt: future }],
|
||||
resolvedThreadsHidden: 0,
|
||||
@@ -356,7 +366,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
|
||||
});
|
||||
|
||||
it('no new comments => tool output is byte-identical AND the model sees no signal', async () => {
|
||||
const fakeClient: Partial<DocmostClientLike> = {
|
||||
const fakeClient: FakeDocmostClient = {
|
||||
getPage: async () => ({
|
||||
data: { title: 'T', content: 'body' },
|
||||
success: true,
|
||||
@@ -372,7 +382,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
|
||||
});
|
||||
|
||||
it('injection-safety: a malicious page title cannot forge a second signal', async () => {
|
||||
const fakeClient: Partial<DocmostClientLike> = {
|
||||
const fakeClient: FakeDocmostClient = {
|
||||
getPage: async () => ({
|
||||
data: { title: 'body-title', content: 'body' },
|
||||
success: true,
|
||||
|
||||
@@ -2,266 +2,92 @@ import { createHash } from 'node:crypto';
|
||||
import { existsSync, readFileSync } from 'node:fs';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
import type { DocmostClient, SharedToolSpec } from '@docmost/mcp';
|
||||
|
||||
// Re-export SharedToolSpec so downstream server modules keep a single import
|
||||
// path (they import it from this loader). The shape is DERIVED from the package
|
||||
// entry, not re-declared here — see the import above (issue #446).
|
||||
export type { SharedToolSpec } from '@docmost/mcp';
|
||||
|
||||
/**
|
||||
* Minimal structural type for the `DocmostClient` class we consume from the
|
||||
* 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.
|
||||
* The exact set of `DocmostClient` methods the per-user in-app tool adapter
|
||||
* consumes. This is the AUTHORITATIVE list of the client surface the server
|
||||
* depends on; the adapter calls these methods POSITIONALLY, so this set is what
|
||||
* the derived type below type-checks against the real class (issue #446).
|
||||
*/
|
||||
export interface DocmostClientLike {
|
||||
type DocmostClientMethod =
|
||||
// --- read ---
|
||||
search(
|
||||
query: string,
|
||||
spaceId?: string,
|
||||
limit?: number,
|
||||
): Promise<{ items: unknown[]; success: boolean }>;
|
||||
getPage(
|
||||
pageId: string,
|
||||
): Promise<{ data: Record<string, unknown>; success: boolean }>;
|
||||
// Light raw page info (`/pages/info`): title + slugId + ProseMirror content,
|
||||
// WITHOUT the Markdown render / subpage expansion getPage does. Used by the
|
||||
// comment-signal probe to read just the page title on a hit.
|
||||
getPageRaw(pageId: string): Promise<Record<string, unknown> | null>;
|
||||
getWorkspace(): Promise<{ data: Record<string, unknown>; success: boolean }>;
|
||||
getSpaces(): Promise<unknown[]>;
|
||||
listPages(
|
||||
spaceId?: string,
|
||||
limit?: number,
|
||||
tree?: boolean,
|
||||
): Promise<unknown[]>;
|
||||
listSidebarPages(spaceId: string, pageId?: string): Promise<unknown[]>;
|
||||
getOutline(pageId: string): Promise<Record<string, unknown>>;
|
||||
getPageJson(pageId: string): Promise<Record<string, unknown>>;
|
||||
getNode(pageId: string, nodeId: string): Promise<Record<string, unknown>>;
|
||||
searchInPage(
|
||||
pageId: string,
|
||||
query: string,
|
||||
opts?: { regex?: boolean; caseSensitive?: boolean; limit?: number },
|
||||
): Promise<Record<string, unknown>>;
|
||||
getTable(pageId: string, tableRef: string): Promise<Record<string, unknown>>;
|
||||
// Returns `{ items, resolvedThreadsHidden }`. DEFAULT (includeResolved unset/
|
||||
// false) hides resolved threads wholesale; pass true for the full feed.
|
||||
listComments(
|
||||
pageId: string,
|
||||
includeResolved?: boolean,
|
||||
): Promise<{ items: unknown[]; resolvedThreadsHidden: number }>;
|
||||
getComment(
|
||||
commentId: string,
|
||||
): Promise<{ data: Record<string, unknown>; success: boolean }>;
|
||||
checkNewComments(
|
||||
spaceId: string,
|
||||
since: string,
|
||||
parentPageId?: string,
|
||||
): Promise<unknown>;
|
||||
listShares(): Promise<unknown[]>;
|
||||
listPageHistory(
|
||||
pageId: string,
|
||||
cursor?: string,
|
||||
): Promise<{ items: unknown[]; nextCursor: string | null }>;
|
||||
getPageHistory(historyId: string): Promise<Record<string, unknown>>;
|
||||
diffPageVersions(
|
||||
pageId: string,
|
||||
from?: string,
|
||||
to?: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
exportPageMarkdown(pageId: string): Promise<string>;
|
||||
| 'search'
|
||||
| 'getPage'
|
||||
| 'getPageRaw'
|
||||
| 'getWorkspace'
|
||||
| 'getSpaces'
|
||||
| 'listPages'
|
||||
| 'listSidebarPages'
|
||||
| 'getOutline'
|
||||
| 'getPageJson'
|
||||
| 'getNode'
|
||||
| 'searchInPage'
|
||||
| 'getTable'
|
||||
| 'listComments'
|
||||
| 'getComment'
|
||||
| 'checkNewComments'
|
||||
| 'listShares'
|
||||
| 'listPageHistory'
|
||||
| 'getPageHistory'
|
||||
| 'diffPageVersions'
|
||||
| 'exportPageMarkdown'
|
||||
// --- write (page) ---
|
||||
createPage(
|
||||
title: string,
|
||||
content: string,
|
||||
spaceId: string,
|
||||
parentPageId?: string,
|
||||
): Promise<{ data: Record<string, unknown>; success: boolean }>;
|
||||
// Markdown content update via the collab path (carries provenance via the
|
||||
// collab-token provider). Optionally also updates the title.
|
||||
updatePage(
|
||||
pageId: string,
|
||||
content: string,
|
||||
title?: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
// Title-only rename via REST.
|
||||
renamePage(
|
||||
pageId: string,
|
||||
title: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
// Move via REST. parentPageId null => move to space root.
|
||||
movePage(
|
||||
pageId: string,
|
||||
parentPageId: string | null,
|
||||
position?: string,
|
||||
): Promise<unknown>;
|
||||
// SOFT delete only (POST /pages/delete with { pageId }). NEVER permanent.
|
||||
deletePage(pageId: string): Promise<unknown>;
|
||||
editPageText(
|
||||
pageId: string,
|
||||
edits: Array<{ find: string; replace: string; replaceAll?: boolean }>,
|
||||
): Promise<Record<string, unknown>>;
|
||||
patchNode(
|
||||
pageId: string,
|
||||
nodeId: string,
|
||||
node: unknown,
|
||||
): Promise<Record<string, unknown>>;
|
||||
insertNode(
|
||||
pageId: string,
|
||||
node: unknown,
|
||||
opts: {
|
||||
position: 'before' | 'after' | 'append';
|
||||
anchorNodeId?: string;
|
||||
anchorText?: string;
|
||||
},
|
||||
): Promise<Record<string, unknown>>;
|
||||
deleteNode(
|
||||
pageId: string,
|
||||
nodeId: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
updatePageJson(
|
||||
pageId: string,
|
||||
doc?: unknown,
|
||||
title?: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
// Attach an author-inline footnote after the first occurrence of anchorText;
|
||||
// numbering + the footnotes list are derived server-side.
|
||||
insertFootnote(
|
||||
pageId: string,
|
||||
anchorText: string,
|
||||
text: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
// Download a web image and insert it into the page (append, or replace/after a
|
||||
// text anchor). `url` is the image http(s) URL.
|
||||
insertImage(
|
||||
pageId: string,
|
||||
url: string,
|
||||
opts?: {
|
||||
align?: 'left' | 'center' | 'right';
|
||||
alt?: string;
|
||||
replaceText?: string;
|
||||
afterText?: string;
|
||||
},
|
||||
): Promise<Record<string, unknown>>;
|
||||
// Swap an existing image (by its attachmentId) for a new one fetched from a web
|
||||
// URL, repointing every reference in the live document.
|
||||
replaceImage(
|
||||
pageId: string,
|
||||
oldAttachmentId: string,
|
||||
url: string,
|
||||
opts?: { align?: 'left' | 'center' | 'right'; alt?: string },
|
||||
): Promise<Record<string, unknown>>;
|
||||
| 'createPage'
|
||||
| 'updatePage'
|
||||
| 'renamePage'
|
||||
| 'movePage'
|
||||
| 'deletePage'
|
||||
| 'editPageText'
|
||||
| 'patchNode'
|
||||
| 'insertNode'
|
||||
| 'deleteNode'
|
||||
| 'updatePageJson'
|
||||
| 'tableInsertRow'
|
||||
| 'tableDeleteRow'
|
||||
| 'tableUpdateCell'
|
||||
| 'copyPageContent'
|
||||
| 'importPageMarkdown'
|
||||
| 'sharePage'
|
||||
| 'unsharePage'
|
||||
| 'restorePageVersion'
|
||||
| 'transformPage'
|
||||
| 'stashPage'
|
||||
// --- write (image / footnote), in-app since #410 ---
|
||||
| 'insertImage'
|
||||
| 'replaceImage'
|
||||
| 'insertFootnote'
|
||||
// --- draw.io diagrams (#423, stage 1) ---
|
||||
// Read a diagram as decoded mxGraph XML (default) or the raw .drawio.svg.
|
||||
// meta.hash is the optimistic-lock key drawioUpdate expects as baseHash.
|
||||
drawioGet(
|
||||
pageId: string,
|
||||
node: string,
|
||||
format?: 'xml' | 'svg',
|
||||
): Promise<Record<string, unknown>>;
|
||||
// Lint mxGraph XML, build the .drawio.svg attachment and insert a drawio node.
|
||||
drawioCreate(
|
||||
pageId: string,
|
||||
where: {
|
||||
position: 'before' | 'after' | 'append';
|
||||
anchorNodeId?: string;
|
||||
anchorText?: string;
|
||||
},
|
||||
xml: string,
|
||||
title?: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
// Optimistic-locked full replacement of a diagram (baseHash from drawioGet).
|
||||
drawioUpdate(
|
||||
pageId: string,
|
||||
node: string,
|
||||
xml: string,
|
||||
baseHash: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
tableInsertRow(
|
||||
pageId: string,
|
||||
tableRef: string,
|
||||
cells: string[],
|
||||
index?: number,
|
||||
): Promise<Record<string, unknown>>;
|
||||
tableDeleteRow(
|
||||
pageId: string,
|
||||
tableRef: string,
|
||||
index: number,
|
||||
): Promise<Record<string, unknown>>;
|
||||
tableUpdateCell(
|
||||
pageId: string,
|
||||
tableRef: string,
|
||||
row: number,
|
||||
col: number,
|
||||
text: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
copyPageContent(
|
||||
sourcePageId: string,
|
||||
targetPageId: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
importPageMarkdown(
|
||||
pageId: string,
|
||||
fullMarkdown: string,
|
||||
): Promise<Record<string, unknown>>;
|
||||
sharePage(
|
||||
pageId: string,
|
||||
searchIndexing?: boolean,
|
||||
): Promise<Record<string, unknown>>;
|
||||
unsharePage(pageId: string): Promise<Record<string, unknown>>;
|
||||
restorePageVersion(historyId: string): Promise<Record<string, unknown>>;
|
||||
// The opts type declares deleteComments? to match the real client signature,
|
||||
// but the agent tool NEVER sets it (comment deletion stays unreachable).
|
||||
transformPage(
|
||||
pageId: string,
|
||||
transformJs: string,
|
||||
opts?: { dryRun?: boolean; deleteComments?: boolean },
|
||||
): Promise<Record<string, unknown>>;
|
||||
| 'drawioGet'
|
||||
| 'drawioCreate'
|
||||
| 'drawioUpdate'
|
||||
// --- write (comment) ---
|
||||
createComment(
|
||||
pageId: string,
|
||||
content: string,
|
||||
type?: 'page' | 'inline',
|
||||
selection?: string,
|
||||
parentCommentId?: string,
|
||||
suggestedText?: string,
|
||||
): Promise<{ data: Record<string, unknown>; success: boolean }>;
|
||||
resolveComment(
|
||||
commentId: string,
|
||||
resolved: boolean,
|
||||
): Promise<Record<string, unknown>>;
|
||||
// Serialize a page + mirror its internal images into the blob sandbox; returns
|
||||
// ONLY a short anonymous URL (the body never enters the model context).
|
||||
stashPage(pageId: string): Promise<{
|
||||
uri: string;
|
||||
sha256: string;
|
||||
size: number;
|
||||
images: { mirrored: number; failed: number };
|
||||
}>;
|
||||
}
|
||||
| 'createComment'
|
||||
| 'resolveComment';
|
||||
|
||||
/**
|
||||
* The client surface the per-user tool adapter consumes, DERIVED from the real
|
||||
* `DocmostClient` type in `@docmost/mcp` (issue #446, restored #294 debt). This
|
||||
* replaces the former hand-mirror of ~45 method signatures.
|
||||
*
|
||||
* `import type` (above) is fully ERASED at compile time, so nothing is actually
|
||||
* imported from the ESM-only package at runtime — the server still loads the
|
||||
* class through the dynamic `import()` trick in `loadDocmostMcp` below; this is
|
||||
* purely a compile-time type. Deriving via `Pick` means a parameter reorder or a
|
||||
* type change to any of these methods in `client.ts` now becomes a SERVER
|
||||
* COMPILE ERROR at the positional call sites in ai-chat-tools.service.ts,
|
||||
* instead of a silent runtime "wrong argument" failure inside an agent tool.
|
||||
*
|
||||
* This made the old name-only drift-guard test
|
||||
* (packages/mcp/test/unit/client-host-contract.test.mjs) redundant — tsc now
|
||||
* enforces both names AND signatures — so that test was removed.
|
||||
*/
|
||||
export type DocmostClientLike = Pick<DocmostClient, DocmostClientMethod>;
|
||||
|
||||
export type DocmostClientConfig = {
|
||||
apiUrl: string;
|
||||
@@ -283,32 +109,7 @@ export type DocmostClientConfig = {
|
||||
};
|
||||
|
||||
export interface DocmostClientCtor {
|
||||
new (config: DocmostClientConfig): DocmostClientLike;
|
||||
}
|
||||
|
||||
/**
|
||||
* Local hand-mirror of the `SharedToolSpec` shape exported from
|
||||
* `@docmost/mcp` (packages/mcp/src/tool-specs.ts). Same approach as
|
||||
* `DocmostClientLike`: we do not import the ESM package's types directly across
|
||||
* the CJS/ESM boundary. The registry itself has no runtime deps, but keeping the
|
||||
* type local avoids coupling the server build to the package's type surface.
|
||||
*
|
||||
* `buildShape` is intentionally zod-agnostic: it returns a plain ZodRawShape
|
||||
* built with whatever zod namespace the caller passes (the server passes its own
|
||||
* zod v4; the MCP package passes its zod v3). See the registry module comment.
|
||||
*/
|
||||
export interface SharedToolSpec {
|
||||
mcpName: string;
|
||||
inAppKey: string;
|
||||
description: string;
|
||||
// Deferred-tool metadata (#332). Optional in this mirror so an older/stale
|
||||
// @docmost/mcp build (pre-#332) still type-checks; the in-app catalog builder
|
||||
// reads them defensively. The external /mcp server ignores both fields.
|
||||
tier?: 'core' | 'deferred';
|
||||
catalogLine?: string;
|
||||
// Loose `z` on purpose: the registry is zod-agnostic so the server can pass
|
||||
// its own zod (v4) and the MCP package its own (v3) into the same builder.
|
||||
buildShape?: (z: any) => Record<string, unknown>;
|
||||
new (config: DocmostClientConfig): DocmostClient;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -5,9 +5,16 @@
|
||||
"private": true,
|
||||
"type": "module",
|
||||
"main": "./build/index.js",
|
||||
"types": "./build/index.d.ts",
|
||||
"exports": {
|
||||
".": "./build/index.js",
|
||||
"./http": "./build/http.js"
|
||||
".": {
|
||||
"types": "./build/index.d.ts",
|
||||
"default": "./build/index.js"
|
||||
},
|
||||
"./http": {
|
||||
"types": "./build/http.d.ts",
|
||||
"default": "./build/http.js"
|
||||
}
|
||||
},
|
||||
"bin": {
|
||||
"docmost-mcp": "./build/stdio.js"
|
||||
|
||||
@@ -1,224 +0,0 @@
|
||||
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 (deleteComment/updateComment) that this guard does NOT track — the
|
||||
// MCP transport's own typecheck covers those. insertImage/replaceImage/insertFootnote
|
||||
// were MCP-only but are now in-app-consumed too (#410), so they ARE tracked below. 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",
|
||||
"getPageRaw",
|
||||
"getWorkspace",
|
||||
"getSpaces",
|
||||
"listPages",
|
||||
"listSidebarPages",
|
||||
"getOutline",
|
||||
"getPageJson",
|
||||
"getNode",
|
||||
"searchInPage",
|
||||
"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",
|
||||
"stashPage",
|
||||
// write (image / footnote) — MCP-only until #410 promoted them to in-app tools
|
||||
"insertImage",
|
||||
"replaceImage",
|
||||
"insertFootnote",
|
||||
// draw.io diagrams (#423, stage 1) — read + create + optimistic-locked update
|
||||
"drawioGet",
|
||||
"drawioCreate",
|
||||
"drawioUpdate",
|
||||
// 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).`,
|
||||
);
|
||||
});
|
||||
@@ -5,6 +5,8 @@
|
||||
"moduleResolution": "Node16",
|
||||
"outDir": "./build",
|
||||
"rootDir": "./src",
|
||||
"declaration": true,
|
||||
"declarationMap": true,
|
||||
"strict": true,
|
||||
"esModuleInterop": true,
|
||||
"skipLibCheck": true,
|
||||
|
||||
Reference in New Issue
Block a user