Compare commits
7 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 3411bda2d1 | |||
| e670f7498a | |||
| bec2156e96 | |||
| a49872444e | |||
| ffc38ea2ca | |||
| 2f23cf4b65 | |||
| d287c15db4 |
@@ -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
|
||||
|
||||
|
||||
@@ -27,10 +27,12 @@ import type { DocmostClientLike } from './docmost-client.loader';
|
||||
*/
|
||||
|
||||
describe('tool tier metadata (#332)', () => {
|
||||
it('core set is the documented 13 + searchInPage + insertFootnote (15)', () => {
|
||||
expect(CORE_TOOL_KEYS).toHaveLength(15);
|
||||
it('core set is the documented 13 + searchInPage + insertFootnote + getTree + getPageContext (17, #443)', () => {
|
||||
expect(CORE_TOOL_KEYS).toHaveLength(17);
|
||||
expect(CORE_TOOL_SET.has('searchInPage')).toBe(true); // #330, promoted to core
|
||||
expect(CORE_TOOL_SET.has('insertFootnote')).toBe(true); // #410, promoted to core
|
||||
expect(CORE_TOOL_SET.has('getTree')).toBe(true); // #443, promoted to core
|
||||
expect(CORE_TOOL_SET.has('getPageContext')).toBe(true); // #443, promoted to core
|
||||
// loadTools is a meta-tool, not a normal core key.
|
||||
expect(CORE_TOOL_SET.has(LOAD_TOOLS_NAME)).toBe(false);
|
||||
});
|
||||
|
||||
@@ -39,12 +39,14 @@ export interface ToolCatalogEntry {
|
||||
|
||||
/**
|
||||
* CORE (always-active) in-app tool keys — 13 frequent/tiny tools + `searchInPage`
|
||||
* (#330) + `insertFootnote` (#410). `searchInPage` is core because it is frequent
|
||||
* for the editorial roles this feature targets; `insertFootnote` is core so the
|
||||
* footnote tool is NOT hidden while its natural sibling `editPageText` is always
|
||||
* active (that asymmetry is exactly what pushed the agent to write literal
|
||||
* `^[...]`). `loadTools` is active too but is not a normal tool key (it is added
|
||||
* to activeTools separately).
|
||||
* (#330) + `insertFootnote` (#410) + `getTree`/`getPageContext` (#443).
|
||||
* `searchInPage` is core because it is frequent for the editorial roles this
|
||||
* feature targets; `insertFootnote` is core so the footnote tool is NOT hidden
|
||||
* while its natural sibling `editPageText` is always active (that asymmetry is
|
||||
* exactly what pushed the agent to write literal `^[...]`). `getTree` and
|
||||
* `getPageContext` are the single-call navigation/lookup tools — core so the
|
||||
* agent never has to loadTools just to orient itself. `loadTools` is active too
|
||||
* but is not a normal tool key (it is added to activeTools separately).
|
||||
*/
|
||||
export const CORE_TOOL_KEYS = [
|
||||
'searchPages',
|
||||
@@ -66,6 +68,11 @@ export const CORE_TOOL_KEYS = [
|
||||
// #410 insertFootnote — core so pinpoint citations to already-written text
|
||||
// don't degrade into literal `^[...]`; kept symmetric with editPageText.
|
||||
'insertFootnote',
|
||||
// #443 getTree + getPageContext — cheap single-call navigation/lookup tools
|
||||
// (the core listPages even points to getTree); core so the agent never has
|
||||
// to loadTools just to orient itself.
|
||||
'getTree',
|
||||
'getPageContext',
|
||||
] as const;
|
||||
|
||||
/** O(1) membership test for the core tier. */
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
+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) };
|
||||
}
|
||||
|
||||
@@ -316,7 +316,8 @@ async function main() {
|
||||
const [idA, idB, idC] = seedIds;
|
||||
|
||||
// patchNode: replace the middle paragraph; siblings' ids must be unchanged.
|
||||
await client.patchNode(nid, idB, mkPara(idB, "Bravo PATCHED."));
|
||||
// #413 XOR input: the raw ProseMirror node goes under the `node` key.
|
||||
await client.patchNode(nid, idB, { node: mkPara(idB, "Bravo PATCHED.") });
|
||||
await new Promise((r) => setTimeout(r, 16000));
|
||||
const afterPatch = (await client.getPageJson(nid)).content;
|
||||
const patchText = JSON.stringify(afterPatch);
|
||||
@@ -327,7 +328,7 @@ async function main() {
|
||||
// insertNode: place a new block after the first paragraph.
|
||||
await client.insertNode(
|
||||
nid,
|
||||
mkPara("nodeops-ins", "Inserted paragraph."),
|
||||
{ node: mkPara("nodeops-ins", "Inserted paragraph.") },
|
||||
{ position: "after", anchorNodeId: idA },
|
||||
);
|
||||
await new Promise((r) => setTimeout(r, 16000));
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
Reference in New Issue
Block a user