Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 2f23cf4b65 | |||
| d287c15db4 | |||
| e19275e96e | |||
| 3a521ada4d | |||
| 576db3c8f9 |
@@ -157,6 +157,12 @@ jobs:
|
||||
- name: Build prosemirror-markdown
|
||||
run: pnpm --filter @docmost/prosemirror-markdown build
|
||||
|
||||
# docmost-client.loader.ts type-imports from @docmost/mcp (issue #446); its
|
||||
# build/ is gitignored and `test:e2e` type-checks, so build it here or tsc
|
||||
# fails with TS2307 (mirrors the e2e-mcp / mcp-server-parity jobs).
|
||||
- name: Build mcp
|
||||
run: pnpm --filter @docmost/mcp build
|
||||
|
||||
- name: Run migrations
|
||||
run: pnpm --filter ./apps/server migration:latest
|
||||
|
||||
|
||||
@@ -10,6 +10,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
- **External MCP: `import_page_markdown` removed, `update_page_markdown` added.**
|
||||
The external `/mcp` surface no longer exposes `import_page_markdown` (the
|
||||
round-trip parser for a self-contained *exported* Docmost-Markdown file). In
|
||||
its place it now exposes **`update_page_markdown`** — a plain-Markdown
|
||||
full-body replace (`{pageId, content, title?}`) that pairs with
|
||||
`update_page_json`, re-imports the whole body (block ids regenerate) and
|
||||
parses Docmost-flavoured markdown including `^[...]` inline footnotes.
|
||||
*Migration:* MCP clients that called `import_page_markdown` to overwrite a
|
||||
page's body from Markdown should call `update_page_markdown` instead (pass the
|
||||
markdown as `content`). Round-tripping an exported Docmost-Markdown file with
|
||||
comment anchors/diagrams is no longer available on the external MCP surface;
|
||||
export remains via `export_page_markdown`. The in-app AI agent is unaffected —
|
||||
it keeps both `importPageMarkdown` and the renamed `updatePageMarkdown` (was
|
||||
`updatePageContent`). The total MCP tool count is unchanged (−1 / +1). (#411)
|
||||
|
||||
### Added
|
||||
|
||||
- **Place several images side by side in a row.** A new "Inline (side by
|
||||
|
||||
@@ -75,7 +75,7 @@ const LABELS: Record<
|
||||
searchPages: 'Searched pages',
|
||||
getPage: 'Read page',
|
||||
createPage: 'Created page',
|
||||
updatePageContent: 'Updated page',
|
||||
updatePageMarkdown: 'Updated page',
|
||||
renamePage: 'Renamed page',
|
||||
movePage: 'Moved page',
|
||||
deletePage: 'Deleted page (to trash)',
|
||||
@@ -96,7 +96,7 @@ const LABELS: Record<
|
||||
searchPages: 'Искал по страницам',
|
||||
getPage: 'Прочитал страницу',
|
||||
createPage: 'Создал страницу',
|
||||
updatePageContent: 'Обновил страницу',
|
||||
updatePageMarkdown: 'Обновил страницу',
|
||||
renamePage: 'Переименовал страницу',
|
||||
movePage: 'Переместил страницу',
|
||||
deletePage: 'Удалил страницу (в корзину)',
|
||||
|
||||
@@ -23,7 +23,7 @@ import { SHARED_TOOL_SPECS } from '../../../../../../packages/mcp/src/tool-specs
|
||||
// sync.
|
||||
const mockLoaded = (DocmostClient: loader.DocmostClientCtor) => ({
|
||||
DocmostClient,
|
||||
sharedToolSpecs: SHARED_TOOL_SPECS as Record<string, loader.SharedToolSpec>,
|
||||
sharedToolSpecs: SHARED_TOOL_SPECS as unknown as Record<string, loader.SharedToolSpec>,
|
||||
// Pure no-network draw.io helpers (#424). Type-correct stubs: these tests
|
||||
// never execute the drawio_shapes / drawio_guide tool bodies.
|
||||
searchShapes: (() => []) as unknown as loader.SearchShapesFn,
|
||||
@@ -283,6 +283,7 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
|
||||
const patchNodeCalls: unknown[][] = [];
|
||||
const insertNodeCalls: unknown[][] = [];
|
||||
const updatePageJsonCalls: unknown[][] = [];
|
||||
const updatePageCalls: unknown[][] = [];
|
||||
|
||||
const fakeClient: FakeDocmostClient = {
|
||||
patchNode: (...args: unknown[]) => {
|
||||
@@ -297,6 +298,11 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
|
||||
updatePageJsonCalls.push(args);
|
||||
return Promise.resolve({ ok: true });
|
||||
},
|
||||
// Backs the plain-Markdown full-body-replace tool updatePageMarkdown (#411).
|
||||
updatePage: (...args: unknown[]) => {
|
||||
updatePageCalls.push(args);
|
||||
return Promise.resolve({ success: true });
|
||||
},
|
||||
};
|
||||
|
||||
const tokenServiceStub = {
|
||||
@@ -310,6 +316,7 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
|
||||
patchNodeCalls.length = 0;
|
||||
insertNodeCalls.length = 0;
|
||||
updatePageJsonCalls.length = 0;
|
||||
updatePageCalls.length = 0;
|
||||
jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue(
|
||||
mockLoaded(function () {
|
||||
return fakeClient as DocmostClientLike;
|
||||
@@ -445,6 +452,54 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
|
||||
).rejects.toThrow('content was a string but not valid JSON');
|
||||
expect(updatePageJsonCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
// #411: the plain-Markdown full-body-replace tool is now the shared
|
||||
// `updatePageMarkdown` (was inline `updatePageContent`). It forwards to
|
||||
// client.updatePage(pageId, content, title) -> updatePageContentRealtime ->
|
||||
// markdownToProseMirrorCanonical, so `^[...]` footnotes materialize.
|
||||
it('updatePageMarkdown forwards { pageId, content, title } to client.updatePage', async () => {
|
||||
const tools = await buildTools();
|
||||
await tools.updatePageMarkdown.execute(
|
||||
{ pageId: 'p1', content: 'Body^[a note]', title: 'New title' } as never,
|
||||
{} as never,
|
||||
);
|
||||
expect(updatePageCalls).toHaveLength(1);
|
||||
expect(updatePageCalls[0]).toEqual(['p1', 'Body^[a note]', 'New title']);
|
||||
});
|
||||
|
||||
it('updatePageMarkdown returns the RAW client result in-app (deliberate #411 shape change, documented on the spec)', async () => {
|
||||
const tools = await buildTools();
|
||||
// Registry canonical execute returns client.updatePage's result verbatim.
|
||||
// The old inline tool projected to { pageId, updated }; the rename now
|
||||
// surfaces the raw result (nothing reads the removed `.updated`; the raw
|
||||
// shape carries footnote/verify warnings and matches the on-both-hosts
|
||||
// registry convention). fakeClient.updatePage resolves { success: true }.
|
||||
const result = await tools.updatePageMarkdown.execute(
|
||||
{ pageId: 'p1', content: '# Hi' } as never,
|
||||
{} as never,
|
||||
);
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
|
||||
it('updatePageMarkdown forwards title=undefined when omitted', async () => {
|
||||
const tools = await buildTools();
|
||||
await tools.updatePageMarkdown.execute(
|
||||
{ pageId: 'p1', content: '# Hi' } as never,
|
||||
{} as never,
|
||||
);
|
||||
expect(updatePageCalls[0]).toEqual(['p1', '# Hi', undefined]);
|
||||
});
|
||||
|
||||
// #411 surface split: the plain-Markdown replace tool exists in-app under the
|
||||
// new key; the OLD inline updatePageContent key is gone; importPageMarkdown is
|
||||
// still present IN-APP (only the external MCP surface drops it — asserted in
|
||||
// packages/mcp/test/unit/tool-inventory.test.mjs).
|
||||
it('exposes updatePageMarkdown in-app, no legacy updatePageContent, keeps importPageMarkdown', async () => {
|
||||
const tools = await buildTools();
|
||||
expect(tools.updatePageMarkdown).toBeDefined();
|
||||
expect((tools as Record<string, unknown>).updatePageContent).toBeUndefined();
|
||||
expect(tools.importPageMarkdown).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
|
||||
@@ -307,8 +307,8 @@ export class AiChatToolsService {
|
||||
|
||||
// The in-app toolset. It starts with the tools kept INLINE here for a
|
||||
// documented per-layer reason: an intentional behaviour/schema divergence from
|
||||
// the standalone MCP surface (searchPages' hybrid RRF, updatePageContent's
|
||||
// Markdown write, transformPage's guardrailed shorter schema), a
|
||||
// the standalone MCP surface (searchPages' hybrid RRF,
|
||||
// transformPage's guardrailed shorter schema), a
|
||||
// snake_case/camelCase naming clash the shared registry forbids (getTable vs
|
||||
// the MCP `table_get`), per-request state the registry loop cannot provide
|
||||
// (getCurrentPage reads the resolved openedPage; searchPages closes over the
|
||||
@@ -451,28 +451,13 @@ export class AiChatToolsService {
|
||||
}),
|
||||
|
||||
// --- WRITE tools (all reversible — history/trash; §6.5 / D3) ---
|
||||
|
||||
updatePageContent: tool({
|
||||
description:
|
||||
"Replace a page's body with new Markdown content (and optionally its " +
|
||||
'title). Reversible: the previous version is kept in page history.',
|
||||
inputSchema: modelFriendlyInput({
|
||||
pageId: z.string().describe('The id of the page to update.'),
|
||||
content: z.string().describe('The new page body as Markdown.'),
|
||||
title: z
|
||||
.string()
|
||||
.optional()
|
||||
.describe('Optional new title for the page.'),
|
||||
}),
|
||||
execute: async ({ pageId, content, title }) => {
|
||||
// updatePage mutates the live collab doc -> provenance flows from the
|
||||
// collab-token provider. Returns { success, modified, message, pageId }.
|
||||
const result = (await client.updatePage(pageId, content, title)) as {
|
||||
success?: boolean;
|
||||
};
|
||||
return { pageId, updated: result?.success ?? true };
|
||||
},
|
||||
}),
|
||||
//
|
||||
// NOTE (issue #411): the plain-Markdown full-body-replace tool is no longer
|
||||
// inline here — it moved to @docmost/mcp's SHARED_TOOL_SPECS as
|
||||
// `updatePageMarkdown` (was inline `updatePageContent`) so it registers on
|
||||
// BOTH the external MCP and the in-app agent. The registry loop below adds
|
||||
// it under its inAppKey. importPageMarkdown stays a shared spec too (now
|
||||
// inAppOnly — dropped from the external MCP surface, kept in-app).
|
||||
|
||||
listSidebarPages: tool({
|
||||
description:
|
||||
|
||||
@@ -283,7 +283,7 @@ describe('AiChatToolsService forUser + comment signal (real tracker)', () => {
|
||||
DocmostClient: function () {
|
||||
return fakeClient as DocmostClientLike;
|
||||
} as unknown as loader.DocmostClientCtor,
|
||||
sharedToolSpecs: SHARED_TOOL_SPECS as Record<string, loader.SharedToolSpec>,
|
||||
sharedToolSpecs: SHARED_TOOL_SPECS as unknown as Record<string, loader.SharedToolSpec>,
|
||||
// Wire the REAL factory so the in-app path is exercised end to end.
|
||||
createCommentSignalTracker:
|
||||
createCommentSignalTracker as unknown as loader.CommentSignalTrackerFactory,
|
||||
|
||||
@@ -80,7 +80,7 @@ describe('SHARED_TOOL_SPECS contract parity', () => {
|
||||
|
||||
// Type as the (optional-buildShape) SharedToolSpec; the `satisfies` literal
|
||||
// above otherwise narrows to a union where some members lack buildShape.
|
||||
const specEntries = Object.entries(SHARED_TOOL_SPECS) as Array<
|
||||
const specEntries = Object.entries(SHARED_TOOL_SPECS) as unknown as Array<
|
||||
[string, loader.SharedToolSpec]
|
||||
>;
|
||||
|
||||
|
||||
@@ -123,7 +123,7 @@ describe('deferred catalog ↔ live forUser() toolset partition (#332, F3)', ()
|
||||
DocmostClient: function () {
|
||||
return {} as DocmostClientLike;
|
||||
} as unknown as loader.DocmostClientCtor,
|
||||
sharedToolSpecs: SHARED_TOOL_SPECS as Record<string, loader.SharedToolSpec>,
|
||||
sharedToolSpecs: SHARED_TOOL_SPECS as unknown as Record<string, loader.SharedToolSpec>,
|
||||
// Pure no-network draw.io helpers (#424); tool bodies are never executed here.
|
||||
searchShapes: (() => []) as unknown as loader.SearchShapesFn,
|
||||
getGuideSection: (() => ({
|
||||
|
||||
@@ -124,12 +124,9 @@ export const INLINE_TOOL_TIERS: Record<
|
||||
// --- deferred inline ---
|
||||
// NOTE: createPage, renamePage, movePage, deletePage, updatePageJson and
|
||||
// exportPageMarkdown moved to @docmost/mcp's SHARED_TOOL_SPECS (#294); they
|
||||
// carry their own deferred tier + catalogLine there.
|
||||
updatePageContent: {
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
"updatePageContent — replace a page's body (and optionally title) with new Markdown.",
|
||||
},
|
||||
// carry their own deferred tier + catalogLine there. updatePageContent moved
|
||||
// there too as updatePageMarkdown (#411) — a shared registry spec now, so it
|
||||
// is no longer an inline tier entry.
|
||||
listSidebarPages: {
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
|
||||
@@ -182,6 +182,7 @@ describe('AiChatService run-stream attach [integration]', () => {
|
||||
{
|
||||
isAiChatDeferredToolsEnabled: () => false,
|
||||
isAiChatResumableStreamEnabled: () => true,
|
||||
isAiChatFinalStepLockdownEnabled: () => false,
|
||||
} as any,
|
||||
registry,
|
||||
);
|
||||
@@ -499,6 +500,7 @@ describe('AiChatService run-stream attach [integration]', () => {
|
||||
{
|
||||
isAiChatDeferredToolsEnabled: () => false,
|
||||
isAiChatResumableStreamEnabled: () => true,
|
||||
isAiChatFinalStepLockdownEnabled: () => false,
|
||||
} as any,
|
||||
registry,
|
||||
);
|
||||
|
||||
@@ -150,7 +150,7 @@ describe('AiChatService.stream [integration]', () => {
|
||||
{} as any, // pageAccess (idem)
|
||||
// environment (#332): keep deferred tool loading OFF for this lifecycle
|
||||
// harness so the toolset/behavior is exactly as before.
|
||||
{ isAiChatDeferredToolsEnabled: () => false } as any,
|
||||
{ isAiChatDeferredToolsEnabled: () => false, isAiChatFinalStepLockdownEnabled: () => false } as any,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -378,7 +378,7 @@ describe('AiChatService.stream [integration]', () => {
|
||||
{} as any,
|
||||
{} as any,
|
||||
// #332: deferred tool loading ON — the property under test.
|
||||
{ isAiChatDeferredToolsEnabled: () => true } as any,
|
||||
{ isAiChatDeferredToolsEnabled: () => true, isAiChatFinalStepLockdownEnabled: () => false } as any,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
+15
-10
@@ -155,6 +155,10 @@ All 41 tools, grouped by what you'd reach for them.
|
||||
- **`update_page_json`** — Replace a page's entire content with a ProseMirror document
|
||||
(bulk rewrites, or when nodes lack ids). `content` is optional — omit it to update only
|
||||
the title. Keeps the block ids you pass in, so heading anchors and history stay stable.
|
||||
- **`update_page_markdown`** — Replace a page's body (and optionally its title) with new
|
||||
**plain Markdown**. The whole body is re-imported (block ids regenerate — for surgical or
|
||||
id-preserving edits prefer `edit_page_text` / `patch_node` / `update_page_json`).
|
||||
Docmost-flavoured markdown is parsed, including `^[...]` inline footnotes.
|
||||
- **`docmost_transform`** — The agent-native editing interface: instead of retyping a
|
||||
document, the agent **writes a function that fixes it**. Edit a page by running an
|
||||
arbitrary **`(doc, ctx) => doc` JavaScript transform** against its *live* ProseMirror
|
||||
@@ -184,13 +188,13 @@ All 41 tools, grouped by what you'd reach for them.
|
||||
|
||||
- **`export_page_markdown`** — Export a page to a single self-contained, **lossless
|
||||
Docmost-flavoured Markdown** file: a meta header, the body with inline comment anchors
|
||||
and diagrams, and a trailing comments-thread block. Built for a download → edit body →
|
||||
`import_page_markdown` round-trip that preserves everything, including comment highlights.
|
||||
- **`import_page_markdown`** — Replace a page's content from a Docmost-flavoured Markdown
|
||||
file produced by `export_page_markdown`, restoring comment-highlight anchors and diagrams
|
||||
from their inline HTML. (Comment *threads* in the file are not re-created on the server —
|
||||
only the page body and inline comment marks are written; manage threads via the comment
|
||||
tools/UI.)
|
||||
and diagrams, and a trailing comments-thread block. To replace a page's body from plain
|
||||
authoring Markdown, use `update_page_markdown`.
|
||||
|
||||
> **Removed in this release:** `import_page_markdown` (the round-trip parser for an
|
||||
> exported Docmost-Markdown file) is **no longer exposed on the external MCP surface**.
|
||||
> To replace a page's body from Markdown, use **`update_page_markdown`** (plain Markdown
|
||||
> body replace). See the CHANGELOG for the migration note.
|
||||
|
||||
### Images
|
||||
|
||||
@@ -256,7 +260,8 @@ so capable clients steer the model automatically.
|
||||
`delete_node`, addressing the node by its `attrs.id` from `get_page_json`.
|
||||
- **Images**: `insert_image` / `replace_image`.
|
||||
- **A new page**: `create_page`.
|
||||
- **Bulk rewrite, or nodes without ids**: `update_page_json`.
|
||||
- **Bulk rewrite, or nodes without ids**: `update_page_json` (ProseMirror) or
|
||||
`update_page_markdown` (plain Markdown body replace).
|
||||
- **Multi-step / scripted rewrite** (renumbering, footnotes, coordinated edits):
|
||||
`docmost_transform` — preview with `dryRun`, then apply.
|
||||
- **Copy a whole page's content from another page** (server-side): `copy_page_content`.
|
||||
@@ -269,8 +274,8 @@ so capable clients steer the model automatically.
|
||||
`get_node`.
|
||||
- **Tables** (add/remove a row, set a cell): `table_get` / `table_insert_row` /
|
||||
`table_delete_row` / `table_update_cell`.
|
||||
- **Round-trip a page as Markdown** (download, edit, re-upload losslessly with comments):
|
||||
`export_page_markdown` / `import_page_markdown`.
|
||||
- **Export a page as self-contained Markdown** (with comment anchors): `export_page_markdown`.
|
||||
- **Replace a page's body from Markdown**: `update_page_markdown`.
|
||||
|
||||
---
|
||||
|
||||
|
||||
+15
-11
@@ -161,6 +161,10 @@ Docmost-MCP не сочетают:
|
||||
перезаписи или когда у узлов нет id). `content` опционален — опустите его, чтобы изменить
|
||||
только заголовок. Сохраняет переданные id блоков, поэтому якоря заголовков и история
|
||||
остаются стабильными.
|
||||
- **`update_page_markdown`** — Заменить тело страницы (и опционально заголовок) новым
|
||||
**обычным Markdown**. Всё тело переимпортируется (id блоков перегенерируются — для
|
||||
хирургических правок или сохранения id используйте `edit_page_text` / `patch_node` /
|
||||
`update_page_json`). Markdown в диалекте Docmost разбирается, включая inline-сноски `^[...]`.
|
||||
- **`docmost_transform`** — Агентоориентированный интерфейс редактирования: вместо
|
||||
перепечатывания документа агент **пишет функцию, которая его чинит**. Редактирует
|
||||
страницу, запуская произвольный **JS-трансформ `(doc, ctx) => doc`** на её *живом*
|
||||
@@ -189,14 +193,13 @@ Docmost-MCP не сочетают:
|
||||
|
||||
- **`export_page_markdown`** — Экспортировать страницу в один самодостаточный, **lossless
|
||||
Markdown в диалекте Docmost**: мета-заголовок, тело с inline-якорями комментариев и
|
||||
диаграммами и завершающий блок тредов комментариев. Рассчитан на цикл «скачать →
|
||||
отредактировать тело → `import_page_markdown`», сохраняющий всё, включая выделения
|
||||
комментариев.
|
||||
- **`import_page_markdown`** — Заменить контент страницы из Markdown-файла в диалекте
|
||||
Docmost, созданного `export_page_markdown`, восстанавливая якоря-выделения комментариев и
|
||||
диаграммы из их inline-HTML. (Треды комментариев из файла не пересоздаются на сервере —
|
||||
записываются только тело страницы и inline-марки комментариев; тредами управляйте через
|
||||
инструменты/UI комментариев.)
|
||||
диаграммами и завершающий блок тредов комментариев. Чтобы заменить тело страницы из
|
||||
обычного авторского Markdown, используйте `update_page_markdown`.
|
||||
|
||||
> **Удалено в этом релизе:** `import_page_markdown` (парсер round-trip для
|
||||
> экспортированного Docmost-Markdown-файла) **больше не отдаётся на внешней MCP-поверхности**.
|
||||
> Чтобы заменить тело страницы из Markdown, используйте **`update_page_markdown`** (замена
|
||||
> тела обычным Markdown). См. заметку о миграции в CHANGELOG.
|
||||
|
||||
### Изображения
|
||||
|
||||
@@ -263,7 +266,8 @@ Docmost-MCP не сочетают:
|
||||
`delete_node`, адресуя узел по его `attrs.id` из `get_page_json`.
|
||||
- **Изображения**: `insert_image` / `replace_image`.
|
||||
- **Новая страница**: `create_page`.
|
||||
- **Массовая перезапись или узлы без id**: `update_page_json`.
|
||||
- **Массовая перезапись или узлы без id**: `update_page_json` (ProseMirror) или
|
||||
`update_page_markdown` (замена тела обычным Markdown).
|
||||
- **Многошаговая / скриптовая перезапись** (перенумерация, сноски, согласованные правки):
|
||||
`docmost_transform` — предпросмотр через `dryRun`, затем применение.
|
||||
- **Скопировать контент целой страницы из другой** (на стороне сервера):
|
||||
@@ -278,8 +282,8 @@ Docmost-MCP не сочетают:
|
||||
→ `get_node`.
|
||||
- **Таблицы** (добавить/удалить строку, задать ячейку): `table_get` / `table_insert_row` /
|
||||
`table_delete_row` / `table_update_cell`.
|
||||
- **Round-trip страницы через Markdown** (скачать, отредактировать, залить обратно без
|
||||
потерь, с комментариями): `export_page_markdown` / `import_page_markdown`.
|
||||
- **Экспорт страницы в самодостаточный Markdown** (с якорями комментариев): `export_page_markdown`.
|
||||
- **Заменить тело страницы из Markdown**: `update_page_markdown`.
|
||||
|
||||
---
|
||||
|
||||
|
||||
+161
-51
@@ -14,7 +14,19 @@
|
||||
* signature.
|
||||
*
|
||||
* If recreateTransform / the changeset throws on a pathological document pair,
|
||||
* we fall back to a coarse block-level text diff so the tool never hard-fails.
|
||||
* OR the pair is too large to diff cheaply (see the size guard below), we fall
|
||||
* back to a coarse block-level text diff so the tool never hard-fails and never
|
||||
* pins the event loop.
|
||||
*
|
||||
* SIZE GUARD (issue #464 — prod CPU-DoS). recreateTransform computes its diff via
|
||||
* rfc6902.createPatch, whose array diff is O(n·m) Levenshtein per array pair and
|
||||
* whose per-run word diff is O(w²); on a large/heavily-changed doc this runs for
|
||||
* seconds-to-hours and starves the whole process (BullMQ, Redis lock renewals,
|
||||
* embeddings). It never THROWS — it just never finishes — so the try/catch below
|
||||
* cannot save us. Because diffDocs runs on EVERY in-app/MCP content edit's verify
|
||||
* report, we PRE-FLIGHT the doc size and route anything above a cheap cap straight
|
||||
* to the coarse fallback (the same shape the catch produces). Same cap+fallback
|
||||
* pattern as the ELK-layout DoS fix (#440 / c917dcc3).
|
||||
*/
|
||||
|
||||
import { Node } from "@tiptap/pm/model";
|
||||
@@ -72,6 +84,56 @@ function countNodes(doc: any, pred: (node: any) => boolean): number {
|
||||
return n;
|
||||
}
|
||||
|
||||
// --- Issue #464: pre-flight size guard for the precise diff ------------------
|
||||
// Defaults are BENCHMARK-derived on the recreateTransform(complexSteps:false,
|
||||
// wordDiffs:true, simplifyDiff:true) pipeline, chosen so the WORST case (a fully
|
||||
// re-written doc — the adversarial shape that drove the incident) keeps the
|
||||
// synchronous block under ~200ms REGARDLESS of input:
|
||||
// - 150 total nodes: worst-case pair ~176ms; the O(node²) array diff crosses
|
||||
// 200ms at ~170 nodes and then explodes super-linearly (400 nodes ~1.3s,
|
||||
// 800 ~5.5s), so cap just below the crossover.
|
||||
// - 12 KiB serialized JSON: an independent axis, because the per-run word diff
|
||||
// is O(words²) — a FEW nodes with very long text runs is dangerous even at a
|
||||
// low node count (17 nodes / ~11 KiB ~176ms, / ~14 KiB ~290ms). A node-light
|
||||
// but byte-heavy doc is still refused.
|
||||
// Either metric over its cap routes to the coarse fallback. Both are env-tunable
|
||||
// for operators who accept more CPU in exchange for exact diffs on larger docs.
|
||||
const DEFAULT_MAX_NODES = 150;
|
||||
const DEFAULT_MAX_BYTES = 12 * 1024;
|
||||
|
||||
/**
|
||||
* Read a positive-integer env override, falling back to `dflt`. Garbage / unset /
|
||||
* non-finite / non-positive all fall back (so the guard can never be accidentally
|
||||
* disabled by a malformed value). Read fresh on every call so a test / operator
|
||||
* can flip the knob without a restart.
|
||||
*/
|
||||
function readPositiveIntEnv(name: string, dflt: number): number {
|
||||
const raw = parseInt(process.env[name] ?? "", 10);
|
||||
return Number.isFinite(raw) && raw > 0 ? raw : dflt;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the pair is too large for the precise (recreateTransform) diff and
|
||||
* must degrade to the coarse fallback. Takes the MAX of the two docs on each
|
||||
* metric so an ASYMMETRIC pair (a small new doc vs a huge old doc, or vice
|
||||
* versa) — which still explodes rfc6902 — is caught. Cheap: one node walk +
|
||||
* one JSON.stringify per doc, both O(size).
|
||||
*/
|
||||
function exceedsDiffSizeGuard(oldDoc: any, newDoc: any): boolean {
|
||||
const maxNodes = readPositiveIntEnv("MCP_DIFF_MAX_NODES", DEFAULT_MAX_NODES);
|
||||
const maxBytes = readPositiveIntEnv("MCP_DIFF_MAX_BYTES", DEFAULT_MAX_BYTES);
|
||||
const nodes = Math.max(
|
||||
countNodes(oldDoc, () => true),
|
||||
countNodes(newDoc, () => true),
|
||||
);
|
||||
if (nodes > maxNodes) return true;
|
||||
const bytes = Math.max(
|
||||
JSON.stringify(oldDoc)?.length ?? 0,
|
||||
JSON.stringify(newDoc)?.length ?? 0,
|
||||
);
|
||||
return bytes > maxBytes;
|
||||
}
|
||||
|
||||
/**
|
||||
* Count UNIQUE links in a JSON doc by their `href`. A single link can be split
|
||||
* across several adjacent text runs (e.g. a "link+bold" run followed by a "link"
|
||||
@@ -226,6 +288,81 @@ function coarseDiff(oldDoc: any, newDoc: any): DiffChange[] {
|
||||
return changes;
|
||||
}
|
||||
|
||||
/** Accumulated textual changes plus their derived char/block tallies. */
|
||||
interface DiffTally {
|
||||
changes: DiffChange[];
|
||||
inserted: number;
|
||||
deleted: number;
|
||||
changedBlocks: Set<string>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Produce the coarse-fallback tally for a pair. This is the SINGLE source of the
|
||||
* `fellBack:true` result shape, shared by BOTH degrade paths in diffDocs (the
|
||||
* pre-flight size guard and the recreateTransform catch) so they behave and
|
||||
* report identically.
|
||||
*/
|
||||
function coarseDiffTally(oldDoc: any, newDoc: any): DiffTally {
|
||||
const changes = coarseDiff(oldDoc, newDoc);
|
||||
let inserted = 0;
|
||||
let deleted = 0;
|
||||
const changedBlocks = new Set<string>();
|
||||
for (const c of changes) {
|
||||
if (c.op === "insert") inserted += c.text.length;
|
||||
else deleted += c.text.length;
|
||||
if (c.block) changedBlocks.add(c.op[0] + ":" + c.block);
|
||||
}
|
||||
return { changes, inserted, deleted, changedBlocks };
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute the PRECISE tally via the recreateTransform pipeline. Callers MUST
|
||||
* gate this behind the size guard (it can block the event loop for a large pair)
|
||||
* and wrap it in try/catch (a pathological pair can throw); on either the guard
|
||||
* or a throw, use `coarseDiffTally` instead. Kept as a sibling of
|
||||
* `coarseDiffTally` so both produce the same `DiffTally` shape.
|
||||
*/
|
||||
function preciseDiffTally(oldDocJson: any, newDocJson: any): DiffTally {
|
||||
const oldNode = Node.fromJSON(docmostSchema, oldDocJson);
|
||||
const newNode = Node.fromJSON(docmostSchema, newDocJson);
|
||||
const tr = recreateTransform(oldNode, newNode, {
|
||||
complexSteps: false,
|
||||
wordDiffs: true,
|
||||
simplifyDiff: true,
|
||||
});
|
||||
const changeSet = ChangeSet.create(oldNode).addSteps(tr.doc, tr.mapping.maps, []);
|
||||
const simplified = simplifyChanges(changeSet.changes, newNode);
|
||||
|
||||
const changes: DiffChange[] = [];
|
||||
let inserted = 0;
|
||||
let deleted = 0;
|
||||
const changedBlocks = new Set<string>();
|
||||
|
||||
for (const change of simplified) {
|
||||
// Deleted text lives in the OLD doc coordinate range [fromA, toA).
|
||||
if (change.toA > change.fromA) {
|
||||
const text = oldNode.textBetween(change.fromA, change.toA, "\n", " ");
|
||||
if (text.length > 0) {
|
||||
deleted += text.length;
|
||||
const block = blockContextAt(oldNode, change.fromA);
|
||||
changes.push({ op: "delete", block, text });
|
||||
if (block) changedBlocks.add("d:" + block);
|
||||
}
|
||||
}
|
||||
// Inserted text lives in the NEW doc coordinate range [fromB, toB).
|
||||
if (change.toB > change.fromB) {
|
||||
const text = newNode.textBetween(change.fromB, change.toB, "\n", " ");
|
||||
if (text.length > 0) {
|
||||
inserted += text.length;
|
||||
const block = blockContextAt(newNode, change.fromB);
|
||||
changes.push({ op: "insert", block, text });
|
||||
if (block) changedBlocks.add("i:" + block);
|
||||
}
|
||||
}
|
||||
}
|
||||
return { changes, inserted, deleted, changedBlocks };
|
||||
}
|
||||
|
||||
/** Build the human-readable unified-ish markdown summary. */
|
||||
function renderMarkdown(
|
||||
result: Omit<DiffResult, "markdown">,
|
||||
@@ -276,66 +413,39 @@ export function diffDocs(
|
||||
newDocJson: any,
|
||||
notesHeading: string = "Примечания переводчика",
|
||||
): DiffResult {
|
||||
// computeIntegrity is cheap (linear node walks) and its counts are needed in
|
||||
// BOTH the precise and coarse paths, so it always runs first.
|
||||
const integrity = computeIntegrity(oldDocJson, newDocJson, notesHeading);
|
||||
|
||||
let changes: DiffChange[] = [];
|
||||
let inserted = 0;
|
||||
let deleted = 0;
|
||||
let fellBack = false;
|
||||
const changedBlocks = new Set<string>();
|
||||
let tally: DiffTally;
|
||||
|
||||
try {
|
||||
const oldNode = Node.fromJSON(docmostSchema, oldDocJson);
|
||||
const newNode = Node.fromJSON(docmostSchema, newDocJson);
|
||||
const tr = recreateTransform(oldNode, newNode, {
|
||||
complexSteps: false,
|
||||
wordDiffs: true,
|
||||
simplifyDiff: true,
|
||||
});
|
||||
const changeSet = ChangeSet.create(oldNode).addSteps(
|
||||
tr.doc,
|
||||
tr.mapping.maps,
|
||||
[],
|
||||
);
|
||||
const simplified = simplifyChanges(changeSet.changes, newNode);
|
||||
|
||||
for (const change of simplified) {
|
||||
// Deleted text lives in the OLD doc coordinate range [fromA, toA).
|
||||
if (change.toA > change.fromA) {
|
||||
const text = oldNode.textBetween(change.fromA, change.toA, "\n", " ");
|
||||
if (text.length > 0) {
|
||||
deleted += text.length;
|
||||
const block = blockContextAt(oldNode, change.fromA);
|
||||
changes.push({ op: "delete", block, text });
|
||||
if (block) changedBlocks.add("d:" + block);
|
||||
}
|
||||
}
|
||||
// Inserted text lives in the NEW doc coordinate range [fromB, toB).
|
||||
if (change.toB > change.fromB) {
|
||||
const text = newNode.textBetween(change.fromB, change.toB, "\n", " ");
|
||||
if (text.length > 0) {
|
||||
inserted += text.length;
|
||||
const block = blockContextAt(newNode, change.fromB);
|
||||
changes.push({ op: "insert", block, text });
|
||||
if (block) changedBlocks.add("i:" + block);
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Pathological pair: degrade to a coarse block-level diff so we never throw.
|
||||
// Pre-flight size guard (#464): a too-large pair would make recreateTransform
|
||||
// block the event loop for seconds-to-hours WITHOUT throwing, so route it to
|
||||
// the coarse fallback BEFORE calling recreateTransform at all. Both this path
|
||||
// and the catch below go through coarseDiffTally for an identical `fellBack`
|
||||
// result shape.
|
||||
if (exceedsDiffSizeGuard(oldDocJson, newDocJson)) {
|
||||
fellBack = true;
|
||||
changes = coarseDiff(oldDocJson, newDocJson);
|
||||
for (const c of changes) {
|
||||
if (c.op === "insert") inserted += c.text.length;
|
||||
else deleted += c.text.length;
|
||||
if (c.block) changedBlocks.add(c.op[0] + ":" + c.block);
|
||||
tally = coarseDiffTally(oldDocJson, newDocJson);
|
||||
} else {
|
||||
try {
|
||||
tally = preciseDiffTally(oldDocJson, newDocJson);
|
||||
} catch {
|
||||
// Pathological pair: degrade to a coarse block-level diff so we never throw.
|
||||
fellBack = true;
|
||||
tally = coarseDiffTally(oldDocJson, newDocJson);
|
||||
}
|
||||
}
|
||||
|
||||
const partial: Omit<DiffResult, "markdown"> = {
|
||||
summary: { inserted, deleted, blocksChanged: changedBlocks.size },
|
||||
summary: {
|
||||
inserted: tally.inserted,
|
||||
deleted: tally.deleted,
|
||||
blocksChanged: tally.changedBlocks.size,
|
||||
},
|
||||
integrity,
|
||||
changes,
|
||||
changes: tally.changes,
|
||||
};
|
||||
return { ...partial, markdown: renderMarkdown(partial, fellBack) };
|
||||
}
|
||||
|
||||
@@ -41,10 +41,10 @@ import { SHARED_TOOL_SPECS, SharedToolSpec } from "./tool-specs.js";
|
||||
export const ROUTING_PROSE =
|
||||
"Docmost editing guide — choose the tool by intent. The <tool_inventory> at the end lists every tool with a one-line purpose; the notes below are the routing hints for WHEN to reach for each.\n" +
|
||||
"READ: find a page -> search (workspace-wide full-text); list -> list_pages / list_spaces. Locate blocks and their ids CHEAPLY -> get_outline (compact top-level map; start here, not get_page_json). One block's subtree -> get_node (by attrs.id, or \"#<index>\" for tables, which carry no id). Find every occurrence of a string/regex ON a page (and where each is) -> search_in_page, NOT block-by-block get_node — it returns each hit's node ref + block index + context for a targeted comment. Whole page -> get_page (Markdown, lossy; inline <span data-comment-id> tags are comment anchors — markup, not text) or get_page_json (lossless ProseMirror with block ids). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stash_page (returns a short-lived anonymous URL).\n" +
|
||||
"EDIT: fix wording/typos/numbers -> edit_page_text (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patch_node (by attrs.id from get_outline). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Tables -> table_get / table_update_cell / table_insert_row / table_delete_row (address by \"#<index>\" from get_outline; table nodes have no attrs.id). Images -> insert_image (add from a web URL) / replace_image (swap an existing image). Draw.io diagrams -> drawio_create (create from mxGraph XML and insert), drawio_get (read a diagram as mxGraph XML + a hash), drawio_update (replace a diagram; pass the hash from drawio_get as baseHash for optimistic locking); before authoring a diagram, drawio_shapes (look up verified stencil style-strings so a shape name never renders as an empty box) and drawio_guide (on-demand authoring reference: skeleton/layout/containers/icons-aws/icons-azure), and pass layout:\"elk\" to drawio_create/drawio_update to auto-place nodes. Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace; prefer the granular tools above to avoid resending the whole ~100KB+ document). Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" +
|
||||
"EDIT: fix wording/typos/numbers -> edit_page_text (find/replace inside blocks, no node id needed). Change ONE block (paragraph/heading/callout/etc.) structurally -> patch_node (by attrs.id from get_outline). Add a block -> insert_node (before/after a block by attrs.id or by anchor text, or append). Remove a block -> delete_node (by attrs.id). Tables -> table_get / table_update_cell / table_insert_row / table_delete_row (address by \"#<index>\" from get_outline; table nodes have no attrs.id). Images -> insert_image (add from a web URL) / replace_image (swap an existing image). Draw.io diagrams -> drawio_create (create from mxGraph XML and insert), drawio_get (read a diagram as mxGraph XML + a hash), drawio_update (replace a diagram; pass the hash from drawio_get as baseHash for optimistic locking); before authoring a diagram, drawio_shapes (look up verified stencil style-strings so a shape name never renders as an empty box) and drawio_guide (on-demand authoring reference: skeleton/layout/containers/icons-aws/icons-azure), and pass layout:\"elk\" to drawio_create/drawio_update to auto-place nodes. Footnotes -> insert_footnote. Bulk/structural rewrite -> update_page_json (full ProseMirror replace) or update_page_markdown (full plain-Markdown body replace, re-imported — block ids regenerate); prefer the granular tools above to avoid resending the whole ~100KB+ document. Complex/scripted rewrite (multiple coordinated edits, renumbering) -> docmost_transform: write a JS `(doc, ctx) => doc` transform, preview the diff with dryRun (default), then apply with dryRun:false; ctx.helpers includes commentsToFootnotes for turning inline comments into numbered footnotes.\n" +
|
||||
"PAGES: new -> create_page (Markdown). Rename (title only) -> rename_page. Move -> move_page. Delete -> delete_page (SOFT delete — the page goes to trash and is restorable; nothing is permanent). Copy/replace a page's whole content from another page (server-side, no document through the model) -> copy_page_content. Sharing -> share_page / unshare_page / list_shares; share_page makes the page PUBLICLY accessible — do it only when explicitly asked.\n" +
|
||||
"COMMENTS: create_comment is always inline and requires an EXACT selection — contiguous text from a single block, <=250 chars (fails rather than leaving an unanchored comment); reply to a thread via parentCommentId. Propose a concrete text fix for one-click human approval -> create_comment with suggestedText (the exact plain-text replacement for the selection; the selection must then be UNIQUE in the page — extend it with context if needed); prefer this over editing directly when the change is subjective or needs the author's sign-off. Manage -> list_comments, update_comment, resolve_comment (resolve/reopen, reversible — prefer over delete to close), delete_comment, check_new_comments.\n" +
|
||||
"HISTORY: review what changed -> diff_page_versions (a historyId vs current, or two versions). List saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). Lossless markdown round-trip (download, edit, re-upload, incl. comment anchors) -> export_page_markdown / import_page_markdown.";
|
||||
"HISTORY: review what changed -> diff_page_versions (a historyId vs current, or two versions). List saved versions -> list_page_history. Undo a bad edit -> restore_page_version (writes a past version back as current; itself revertible). Export a page to self-contained Docmost Markdown (with comment anchors) -> export_page_markdown.";
|
||||
|
||||
/**
|
||||
* A single generated inventory line: the tool's registered NAME + a one-line
|
||||
@@ -96,6 +96,7 @@ const TOOL_FAMILY: Record<string, Family> = {
|
||||
insert_node: "EDIT",
|
||||
delete_node: "EDIT",
|
||||
update_page_json: "EDIT",
|
||||
update_page_markdown: "EDIT",
|
||||
table_get: "EDIT",
|
||||
table_update_cell: "EDIT",
|
||||
table_insert_row: "EDIT",
|
||||
@@ -130,7 +131,8 @@ const TOOL_FAMILY: Record<string, Family> = {
|
||||
list_page_history: "HISTORY",
|
||||
restore_page_version: "HISTORY",
|
||||
export_page_markdown: "HISTORY",
|
||||
import_page_markdown: "HISTORY",
|
||||
// import_page_markdown is now inAppOnly (#411) — it is not registered on the
|
||||
// external MCP host, so it no longer appears in the generated inventory.
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -81,6 +81,7 @@ export type DocmostClientLike = Pick<
|
||||
| 'patchNode'
|
||||
| 'insertNode'
|
||||
| 'deleteNode'
|
||||
| 'updatePage'
|
||||
| 'updatePageJson'
|
||||
| 'tableInsertRow'
|
||||
| 'tableDeleteRow'
|
||||
@@ -642,6 +643,12 @@ export const SHARED_TOOL_SPECS = {
|
||||
importPageMarkdown: {
|
||||
mcpName: 'import_page_markdown',
|
||||
inAppKey: 'importPageMarkdown',
|
||||
// IN-APP ONLY (issue #411): the external /mcp surface no longer exposes
|
||||
// import_page_markdown — the registry loop in index.ts skips inAppOnly specs,
|
||||
// so this stays available to the in-app agent (round-tripping an EXPORTED
|
||||
// Docmost-Markdown file) but is removed from the public MCP tool set. Plain
|
||||
// authoring-markdown body replace on the MCP surface is update_page_markdown.
|
||||
inAppOnly: true,
|
||||
description:
|
||||
"Replace a page's content from a self-contained Docmost-flavoured " +
|
||||
'Markdown file produced by the page-Markdown export tool. Restores comment ' +
|
||||
@@ -1119,6 +1126,51 @@ export const SHARED_TOOL_SPECS = {
|
||||
},
|
||||
},
|
||||
|
||||
// Full-body replace from PLAIN Markdown (issue #411). Pairs with
|
||||
// updatePageJson (which takes a ProseMirror document): this one takes a
|
||||
// markdown string and re-imports the whole body. `client.updatePage` runs it
|
||||
// through updatePageContentRealtime -> markdownToProseMirrorCanonical, so
|
||||
// Docmost-flavoured markdown (incl. `^[...]` inline footnotes) is parsed and
|
||||
// canonicalized. Distinct from importPageMarkdown, which re-imports a
|
||||
// self-contained EXPORTED Docmost-Markdown file (with comment anchors +
|
||||
// diagrams); this tool takes ordinary authoring markdown. Shared spec, so the
|
||||
// registry loop registers it on BOTH hosts (external MCP + in-app agent) —
|
||||
// #411 replaced the old inline in-app `updatePageContent` tool with this.
|
||||
updatePageMarkdown: {
|
||||
// snake_case for now; camelCase public MCP naming is the next issue (#412).
|
||||
mcpName: 'update_page_markdown',
|
||||
inAppKey: 'updatePageMarkdown',
|
||||
description:
|
||||
"Replace a page's body with new Markdown content (and optionally its " +
|
||||
'title). The whole body is re-imported from the markdown (block ids ' +
|
||||
'regenerate — for surgical or id-preserving edits use the find/replace, ' +
|
||||
'node-patch or page-JSON tools instead). Docmost-flavoured markdown is ' +
|
||||
'parsed, including `^[...]` inline footnotes. Reversible: the previous ' +
|
||||
'version is kept in page history.',
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
"updatePageMarkdown — replace a page's body (and optionally title) with new Markdown.",
|
||||
buildShape: (z) => ({
|
||||
pageId: z.string().min(1).describe('The id of the page to update.'),
|
||||
content: z.string().describe('The new page body as Markdown.'),
|
||||
title: z
|
||||
.string()
|
||||
.optional()
|
||||
.describe('Optional new title for the page.'),
|
||||
}),
|
||||
// Single canonical execute on BOTH hosts (the tool was in-app only before,
|
||||
// so there is no external-MCP behavior to preserve). NOTE (rename #411): the
|
||||
// old inline in-app tool projected the client result to { pageId, updated };
|
||||
// the registry now returns the raw client result { success, modified,
|
||||
// message, pageId, verify? } instead. Deliberate: no code reads the removed
|
||||
// `.updated` field, the raw result is strictly more informative to the model
|
||||
// (it surfaces footnote/verify warnings), and it matches the on-both-hosts
|
||||
// registry convention. The result-shape change is the ONLY behavior delta of
|
||||
// this rename; the write path (updatePage -> markdown canonicalize) is identical.
|
||||
execute: (client, { pageId, content, title }) =>
|
||||
client.updatePage(pageId as string, content as string, title as string | undefined),
|
||||
},
|
||||
|
||||
exportPageMarkdown: {
|
||||
mcpName: 'export_page_markdown',
|
||||
inAppKey: 'exportPageMarkdown',
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
// footnote-canonical doc. These override the `replacePage` seam (symmetric to the
|
||||
// `mutatePage` seam used by the insert-footnote-wrapper test) to capture the
|
||||
// persisted doc WITHOUT a live Hocuspocus collab socket. Symmetric to the
|
||||
// server-side focus specs for createPage / updatePageContent('replace').
|
||||
// server-side focus specs for createPage / updatePage (markdown 'replace').
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { DocmostClient } from "../../build/client.js";
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
// Issue #464 — prove the size guard SKIPS the recreateTransform pipeline over
|
||||
// the cap, not merely that it returns "coarse". node:test's mock.module needs an
|
||||
// experimental flag the suite does not pass, so instead of a module spy we use a
|
||||
// deterministic BEHAVIORAL proxy that isolates the one variable — the guard:
|
||||
//
|
||||
// Same over-cap pair, run twice:
|
||||
// (a) default caps -> guard trips -> recreateTransform skipped,
|
||||
// (b) caps raised above the doc -> guard OFF -> recreateTransform DOES run.
|
||||
//
|
||||
// The only code path that differs between (a) and (b) is whether
|
||||
// recreateTransform executes. recreateTransform on this pair is O(n²) and takes
|
||||
// SECONDS; the guarded path is a linear coarse diff taking milliseconds. So a
|
||||
// large (a)≪(b) time ratio can ONLY be explained by (a) skipping the transform.
|
||||
// This asserts the skip without depending on mock.module.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { diffDocs } from "../../build/lib/diff.js";
|
||||
|
||||
const t = (text) => ({ type: "text", text });
|
||||
const para = (text) => ({ type: "paragraph", content: text ? [t(text)] : [] });
|
||||
const doc = (children) => ({ type: "doc", content: children });
|
||||
function buildDoc(n, seed) {
|
||||
return doc(
|
||||
Array.from({ length: n }, (_, i) =>
|
||||
para(Array.from({ length: 8 }, (_, w) => `${seed}${i}_${w}`).join(" ")),
|
||||
),
|
||||
);
|
||||
}
|
||||
function clearEnv() {
|
||||
delete process.env.MCP_DIFF_MAX_NODES;
|
||||
delete process.env.MCP_DIFF_MAX_BYTES;
|
||||
}
|
||||
function timed(fn) {
|
||||
const s = performance.now();
|
||||
const out = fn();
|
||||
return { out, ms: performance.now() - s };
|
||||
}
|
||||
|
||||
// A 300-para (~600-node) pair: comfortably over the 150-node default, yet small
|
||||
// enough that the un-guarded recreateTransform still FINISHES (~1-3s) so the
|
||||
// test can time the contrast without hanging.
|
||||
const OLD = buildDoc(300, "a");
|
||||
const NEW = buildDoc(300, "b");
|
||||
|
||||
test("guard skips recreateTransform over-cap (guarded run is far faster than un-guarded)", () => {
|
||||
// (a) Guarded: default caps -> should short-circuit to coarse, near-instant.
|
||||
clearEnv();
|
||||
const guarded = timed(() => diffDocs(OLD, NEW));
|
||||
assert.match(
|
||||
guarded.out.markdown,
|
||||
/coarse block-level diff/,
|
||||
"guarded run must be coarse (guard tripped)",
|
||||
);
|
||||
|
||||
// (b) Un-guarded: raise both caps above the doc so the precise path runs.
|
||||
process.env.MCP_DIFF_MAX_NODES = "1000000";
|
||||
process.env.MCP_DIFF_MAX_BYTES = "100000000";
|
||||
let unguarded;
|
||||
try {
|
||||
unguarded = timed(() => diffDocs(OLD, NEW));
|
||||
} finally {
|
||||
clearEnv();
|
||||
}
|
||||
assert.doesNotMatch(
|
||||
unguarded.out.markdown,
|
||||
/coarse block-level diff/,
|
||||
"with caps raised, the precise recreateTransform path runs",
|
||||
);
|
||||
|
||||
// The precise run executed recreateTransform (O(n²)); the guarded run did not.
|
||||
// Require a large speedup so the ONLY explanation is the skipped transform.
|
||||
assert.ok(
|
||||
guarded.ms * 5 < unguarded.ms,
|
||||
`guarded (${guarded.ms.toFixed(1)}ms) must be >=5x faster than un-guarded ` +
|
||||
`(${unguarded.ms.toFixed(1)}ms); a small gap would mean the transform still ran`,
|
||||
);
|
||||
});
|
||||
|
||||
test("guarded over-cap call stays within the ~200ms event-loop budget", () => {
|
||||
clearEnv();
|
||||
// Best-of-3 to shed GC/JIT noise; the guarded coarse path is a linear walk.
|
||||
let best = Infinity;
|
||||
for (let i = 0; i < 3; i++) best = Math.min(best, timed(() => diffDocs(OLD, NEW)).ms);
|
||||
assert.ok(best < 200, `guarded over-cap diff must be <200ms, was ${best.toFixed(1)}ms`);
|
||||
});
|
||||
@@ -0,0 +1,229 @@
|
||||
// Issue #464 — prod CPU-DoS pre-flight size guard for diffDocs.
|
||||
//
|
||||
// diffDocs synchronously calls recreateTransform (rfc6902) which is O(n·m) in
|
||||
// node count and O(w²) in per-run word count; on a large/heavily-changed doc it
|
||||
// pins the event loop for seconds-to-hours WITHOUT throwing. A pre-flight size
|
||||
// guard routes any doc over MCP_DIFF_MAX_NODES / MCP_DIFF_MAX_BYTES straight to
|
||||
// the coarse fallback (`fellBack:true`), so the sync block stays ~<200ms.
|
||||
//
|
||||
// These tests assert the BEHAVIOR of the guard (fast + coarse-mode + asymmetry +
|
||||
// env knobs). A sibling test (diff-guard-skips-recreate.test.mjs) proves
|
||||
// recreateTransform is skipped over the cap via a behavioral proxy (guarded run
|
||||
// is orders of magnitude faster than the same pair with the caps raised).
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { diffDocs } from "../../build/lib/diff.js";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Builders
|
||||
// ---------------------------------------------------------------------------
|
||||
const t = (text) => ({ type: "text", text });
|
||||
const para = (text) => ({ type: "paragraph", content: text ? [t(text)] : [] });
|
||||
const doc = (children) => ({ type: "doc", content: children });
|
||||
|
||||
/** A doc of `n` paragraphs whose words are seeded from `seed` (fully changeable). */
|
||||
function buildDoc(n, wordsPerPara, seed) {
|
||||
const blocks = [];
|
||||
for (let i = 0; i < n; i++) {
|
||||
const words = [];
|
||||
for (let w = 0; w < wordsPerPara; w++) words.push(`${seed}${i}_${w}`);
|
||||
blocks.push(para(words.join(" ")));
|
||||
}
|
||||
return doc(blocks);
|
||||
}
|
||||
|
||||
/** Reset the env knobs to their unset default between tests. */
|
||||
function clearEnv() {
|
||||
delete process.env.MCP_DIFF_MAX_NODES;
|
||||
delete process.env.MCP_DIFF_MAX_BYTES;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Over-threshold (by node count) -> FAST + coarse mode.
|
||||
// A fully re-written 600-para doc is the worst case that drove the incident;
|
||||
// with the guard it must return in well under the ~200ms budget and in coarse
|
||||
// mode. Without the guard this single call takes multiple SECONDS.
|
||||
// ---------------------------------------------------------------------------
|
||||
test("over-threshold doc falls back to coarse mode and returns fast", () => {
|
||||
clearEnv();
|
||||
// 600 paragraphs -> ~1200 nodes, far over the 150-node default.
|
||||
const oldDoc = buildDoc(600, 8, "a");
|
||||
const newDoc = buildDoc(600, 8, "b");
|
||||
|
||||
const start = performance.now();
|
||||
const r = diffDocs(oldDoc, newDoc);
|
||||
const elapsed = performance.now() - start;
|
||||
|
||||
// Coarse mode is signalled in the markdown note (fellBack path).
|
||||
assert.match(
|
||||
r.markdown,
|
||||
/coarse block-level diff/,
|
||||
"over-threshold pair must use the coarse fallback",
|
||||
);
|
||||
// Budget: the guard makes this near-instant. Generous 1s ceiling to avoid CI
|
||||
// flake while still being ~10x under the multi-second un-guarded cost.
|
||||
assert.ok(
|
||||
elapsed < 1000,
|
||||
`expected fast coarse fallback, took ${elapsed.toFixed(0)}ms`,
|
||||
);
|
||||
// Coarse diff still detects the wholesale change.
|
||||
assert.ok(r.summary.inserted > 0 || r.summary.deleted > 0, "reports changes");
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Under-threshold (small) doc -> precise diff, NOT coarse mode. No regression.
|
||||
// ---------------------------------------------------------------------------
|
||||
test("under-threshold doc uses the precise diff (no fallback note)", () => {
|
||||
clearEnv();
|
||||
const oldDoc = doc([para("Hello world")]);
|
||||
const newDoc = doc([para("Hello brave world")]);
|
||||
const r = diffDocs(oldDoc, newDoc);
|
||||
|
||||
assert.doesNotMatch(
|
||||
r.markdown,
|
||||
/coarse block-level diff/,
|
||||
"a small doc must take the precise path",
|
||||
);
|
||||
// Precise word diff finds exactly the inserted word.
|
||||
const ins = r.changes.find((c) => c.op === "insert");
|
||||
assert.ok(ins && /brave/.test(ins.text), "precise diff isolates the inserted word");
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Asymmetry: a small NEW doc vs a huge OLD doc (and vice versa) still explodes
|
||||
// rfc6902, so max(old,new) must trip the guard in BOTH directions.
|
||||
// ---------------------------------------------------------------------------
|
||||
test("asymmetric pair (huge old, tiny new) falls back to coarse", () => {
|
||||
clearEnv();
|
||||
const hugeOld = buildDoc(600, 8, "a");
|
||||
const tinyNew = doc([para("just one line")]);
|
||||
const r = diffDocs(hugeOld, tinyNew);
|
||||
assert.match(r.markdown, /coarse block-level diff/, "huge-old side must trip the guard");
|
||||
});
|
||||
|
||||
test("asymmetric pair (tiny old, huge new) falls back to coarse", () => {
|
||||
clearEnv();
|
||||
const tinyOld = doc([para("just one line")]);
|
||||
const hugeNew = buildDoc(600, 8, "b");
|
||||
const r = diffDocs(tinyOld, hugeNew);
|
||||
assert.match(r.markdown, /coarse block-level diff/, "huge-new side must trip the guard");
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Byte axis: a FEW nodes but a very large serialized size (long text runs) is
|
||||
// dangerous too (per-run word diff is O(words²)), so the byte cap must trip
|
||||
// independently of the node count.
|
||||
// ---------------------------------------------------------------------------
|
||||
test("node-light but byte-heavy doc falls back on the byte cap", () => {
|
||||
clearEnv();
|
||||
// 5 paragraphs (~11 nodes, well under the node cap) but each a very long run,
|
||||
// pushing the serialized size far over the 12 KiB byte default.
|
||||
const bigRun = (seed) =>
|
||||
doc(
|
||||
Array.from({ length: 5 }, (_, i) =>
|
||||
para(Array.from({ length: 800 }, (_, w) => `${seed}${i}_${w}`).join(" ")),
|
||||
),
|
||||
);
|
||||
const oldDoc = bigRun("a");
|
||||
const newDoc = bigRun("b");
|
||||
// Sanity: node count is under the default node cap, so ONLY the byte cap can
|
||||
// be what trips the guard here.
|
||||
const nodeCount = (d) => {
|
||||
let n = 0;
|
||||
const v = (x) => {
|
||||
if (!x || typeof x !== "object") return;
|
||||
n++;
|
||||
if (Array.isArray(x.content)) for (const c of x.content) v(c);
|
||||
};
|
||||
v(d);
|
||||
return n;
|
||||
};
|
||||
assert.ok(nodeCount(oldDoc) < 150, "node count is under the node cap");
|
||||
assert.ok(JSON.stringify(oldDoc).length > 12 * 1024, "serialized size is over the byte cap");
|
||||
|
||||
const r = diffDocs(oldDoc, newDoc);
|
||||
assert.match(r.markdown, /coarse block-level diff/, "byte cap must trip independently");
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Env override: a very low MCP_DIFF_MAX_NODES forces fallback on a tiny doc,
|
||||
// proving the knob is read fresh and actually gates the diff.
|
||||
// ---------------------------------------------------------------------------
|
||||
test("MCP_DIFF_MAX_NODES override forces fallback on a small doc", () => {
|
||||
clearEnv();
|
||||
const oldDoc = doc([para("Hello world")]);
|
||||
const newDoc = doc([para("Hello brave world")]);
|
||||
|
||||
// Baseline: default caps -> precise diff.
|
||||
assert.doesNotMatch(diffDocs(oldDoc, newDoc).markdown, /coarse block-level diff/);
|
||||
|
||||
// Knob set absurdly low -> even this 4-node doc trips the guard.
|
||||
process.env.MCP_DIFF_MAX_NODES = "1";
|
||||
try {
|
||||
const r = diffDocs(oldDoc, newDoc);
|
||||
assert.match(r.markdown, /coarse block-level diff/, "low node cap forces fallback");
|
||||
} finally {
|
||||
clearEnv();
|
||||
}
|
||||
});
|
||||
|
||||
test("MCP_DIFF_MAX_BYTES override forces fallback on a small doc", () => {
|
||||
clearEnv();
|
||||
const oldDoc = doc([para("Hello world")]);
|
||||
const newDoc = doc([para("Hello brave world")]);
|
||||
|
||||
process.env.MCP_DIFF_MAX_BYTES = "1";
|
||||
try {
|
||||
const r = diffDocs(oldDoc, newDoc);
|
||||
assert.match(r.markdown, /coarse block-level diff/, "low byte cap forces fallback");
|
||||
} finally {
|
||||
clearEnv();
|
||||
}
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Garbage / unset env values fall back to the DEFAULT (the guard can never be
|
||||
// accidentally disabled by a malformed knob). A small doc must still diff
|
||||
// precisely under a garbage cap.
|
||||
// ---------------------------------------------------------------------------
|
||||
test("garbage env values fall back to the default cap (guard not disabled)", () => {
|
||||
clearEnv();
|
||||
const oldDoc = doc([para("Hello world")]);
|
||||
const newDoc = doc([para("Hello brave world")]);
|
||||
|
||||
for (const bad of ["not-a-number", "0", "-5", "", "NaN", "1e999"]) {
|
||||
process.env.MCP_DIFF_MAX_NODES = bad;
|
||||
process.env.MCP_DIFF_MAX_BYTES = bad;
|
||||
// Under the DEFAULT caps this small doc is precise (garbage did not raise
|
||||
// OR disable the cap). "1e999" -> parseInt yields 1 (finite) which is a
|
||||
// valid low cap and would fall back; exclude that from the precise check.
|
||||
const r = diffDocs(oldDoc, newDoc);
|
||||
if (bad === "1e999") {
|
||||
// parseInt("1e999",10) === 1 -> a legit low cap -> fallback. Guard active.
|
||||
assert.match(r.markdown, /coarse block-level diff/);
|
||||
} else {
|
||||
assert.doesNotMatch(
|
||||
r.markdown,
|
||||
/coarse block-level diff/,
|
||||
`garbage value ${JSON.stringify(bad)} must fall back to the default cap`,
|
||||
);
|
||||
}
|
||||
}
|
||||
clearEnv();
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// A large doc that trips the guard must still return the correct INTEGRITY
|
||||
// counts (computeIntegrity runs before the diff and is unaffected by fallback).
|
||||
// ---------------------------------------------------------------------------
|
||||
test("integrity counts are still correct on a guard-tripped (coarse) doc", () => {
|
||||
clearEnv();
|
||||
const image = { type: "image", attrs: { src: "/api/files/a.png" } };
|
||||
const oldDoc = doc([image, ...buildDoc(600, 8, "a").content]);
|
||||
const newDoc = doc([...buildDoc(600, 8, "b").content]); // image removed
|
||||
|
||||
const r = diffDocs(oldDoc, newDoc);
|
||||
assert.match(r.markdown, /coarse block-level diff/, "large pair fell back");
|
||||
assert.deepEqual(r.integrity.images, [1, 0], "integrity is computed regardless of fallback");
|
||||
});
|
||||
@@ -27,7 +27,8 @@ const SRC = join(HERE, "..", "..", "src");
|
||||
/**
|
||||
* Every tool name the MCP server registers, scraped from the SOURCE:
|
||||
* - inline `server.registerTool("name", ...)` calls in index.ts;
|
||||
* - shared specs in tool-specs.ts (`mcpName: 'name'`).
|
||||
* - shared specs in tool-specs.ts (`mcpName: 'name'`), EXCEPT `inAppOnly`
|
||||
* specs, which the registry loop in index.ts SKIPS on the MCP host (#411).
|
||||
* Same two registration mechanisms the old guard covered.
|
||||
*/
|
||||
function registeredToolNames() {
|
||||
@@ -37,8 +38,13 @@ function registeredToolNames() {
|
||||
for (const m of indexSrc.matchAll(/registerTool\(\s*"([a-z0-9_]+)"/g)) {
|
||||
names.add(m[1]);
|
||||
}
|
||||
for (const m of specsSrc.matchAll(/mcpName:\s*['"]([a-z0-9_]+)['"]/g)) {
|
||||
names.add(m[1]);
|
||||
// Each spec is one `{ ... }` block; scrape its mcpName but skip a block that
|
||||
// carries `inAppOnly: true` (not registered on the external MCP host).
|
||||
for (const block of specsSrc.split(/\n\s{2}\w+:\s*\{/)) {
|
||||
const nameMatch = block.match(/mcpName:\s*['"]([a-z0-9_]+)['"]/);
|
||||
if (!nameMatch) continue;
|
||||
if (/inAppOnly:\s*true/.test(block)) continue;
|
||||
names.add(nameMatch[1]);
|
||||
}
|
||||
return names;
|
||||
}
|
||||
@@ -76,6 +82,30 @@ test("the inventory has no phantom tool (every line is a real registered tool)",
|
||||
);
|
||||
});
|
||||
|
||||
// #411: the external MCP surface gains update_page_markdown and LOSES
|
||||
// import_page_markdown (now inAppOnly). The in-app agent still keeps
|
||||
// importPageMarkdown — asserted in the server-side contract spec.
|
||||
test("update_page_markdown is on the MCP surface; import_page_markdown is NOT", () => {
|
||||
const inventory = new Set(buildToolInventoryLines().map((l) => l.name));
|
||||
assert.ok(
|
||||
inventory.has("update_page_markdown"),
|
||||
"update_page_markdown should be registered on the external MCP surface",
|
||||
);
|
||||
assert.ok(
|
||||
!inventory.has("import_page_markdown"),
|
||||
"import_page_markdown must be dropped from the external MCP surface (#411)",
|
||||
);
|
||||
// And the routing prose no longer points MCP clients at it.
|
||||
assert.ok(
|
||||
!ROUTING_PROSE.includes("import_page_markdown"),
|
||||
"ROUTING_PROSE still mentions the removed import_page_markdown",
|
||||
);
|
||||
assert.ok(
|
||||
ROUTING_PROSE.includes("update_page_markdown"),
|
||||
"ROUTING_PROSE should mention update_page_markdown",
|
||||
);
|
||||
});
|
||||
|
||||
test("every inventory line has a non-empty purpose", () => {
|
||||
for (const line of buildToolInventoryLines()) {
|
||||
assert.equal(typeof line.purpose, "string");
|
||||
|
||||
@@ -145,3 +145,36 @@ test("no-arg specs (getWorkspace/listSpaces/listShares) omit buildShape", () =>
|
||||
assert.equal(SHARED_TOOL_SPECS[key].buildShape, undefined, `${key} should be no-arg`);
|
||||
}
|
||||
});
|
||||
|
||||
// #411: plain-Markdown full-body replace tool, paired with updatePageJson.
|
||||
test("updatePageMarkdown spec exists, pairs with updatePageJson, builds { pageId, content, title }", () => {
|
||||
const spec = SHARED_TOOL_SPECS.updatePageMarkdown;
|
||||
assert.ok(spec, "updatePageMarkdown spec missing");
|
||||
assert.equal(spec.mcpName, "update_page_markdown");
|
||||
assert.equal(spec.inAppKey, "updatePageMarkdown");
|
||||
// Registered on BOTH hosts (a shared spec, no inAppOnly/mcpOnly flag).
|
||||
assert.notEqual(spec.inAppOnly, true);
|
||||
assert.notEqual(spec.mcpOnly, true);
|
||||
// Same tier as its JSON sibling.
|
||||
assert.equal(spec.tier, SHARED_TOOL_SPECS.updatePageJson.tier);
|
||||
const shape = spec.buildShape(z);
|
||||
assert.deepEqual(Object.keys(shape).sort(), ["content", "pageId", "title"]);
|
||||
// pageId + content required, title optional.
|
||||
const schema = z.object(shape);
|
||||
assert.doesNotThrow(() => schema.parse({ pageId: "p1", content: "# Hi" }));
|
||||
assert.throws(() => schema.parse({ pageId: "p1" }));
|
||||
// The description must flag the `^[...]` inline-footnote parse path so the
|
||||
// markdown->footnote canonicalization guarantee stays documented (#411).
|
||||
assert.match(spec.description, /\^\[/);
|
||||
});
|
||||
|
||||
// #411: import_page_markdown is dropped from the EXTERNAL MCP surface but stays
|
||||
// available to the in-app agent — encoded as inAppOnly on the shared spec.
|
||||
test("importPageMarkdown spec is inAppOnly (removed from the external MCP surface, kept in-app)", () => {
|
||||
const spec = SHARED_TOOL_SPECS.importPageMarkdown;
|
||||
assert.ok(spec, "importPageMarkdown spec missing");
|
||||
assert.equal(spec.inAppOnly, true);
|
||||
// The spec + its client method are NOT deleted — only hidden from the MCP host.
|
||||
assert.equal(spec.mcpName, "import_page_markdown");
|
||||
assert.equal(spec.inAppKey, "importPageMarkdown");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user