Compare commits

..

1 Commits

55 changed files with 166 additions and 6695 deletions
-69
View File
@@ -62,38 +62,6 @@ jobs:
needs: [test, e2e-server, e2e-mcp, build]
runs-on: ubuntu-latest
timeout-minutes: 30
# Image boot-smoke (issue #476): every other job tests code from the working
# tree, but the :develop IMAGE that watchtower pulls was never actually
# started anywhere (incident classes #353/#452/#361-boot: startup-migrator
# crash-loop, runtime module missing from the image, wrong static-asset
# headers). The services below back a smoke boot of the exact image right
# before it is pushed; a smoke failure blocks the push.
services:
postgres:
# via mirror.gcr.io (Docker Hub pull-through cache; avoids Hub anonymous
# pull rate-limit that randomly fails on shared GitHub runner IPs).
image: mirror.gcr.io/pgvector/pgvector:pg18
env:
POSTGRES_DB: docmost
POSTGRES_USER: docmost
POSTGRES_PASSWORD: docmost
ports:
- 5432:5432
options: >-
--health-cmd "pg_isready -U docmost"
--health-interval 5s
--health-timeout 5s
--health-retries 20
redis:
# via mirror.gcr.io (see postgres note above).
image: mirror.gcr.io/library/redis:7
ports:
- 6379:6379
options: >-
--health-cmd "redis-cli ping"
--health-interval 5s
--health-timeout 5s
--health-retries 20
steps:
- name: Checkout
uses: actions/checkout@v4
@@ -114,37 +82,6 @@ jobs:
id: version
run: echo "value=$(git describe --tags --always)" >> "$GITHUB_OUTPUT"
# Load the image into the local docker daemon so it can be booted (the
# push step below exports straight to the registry and leaves nothing
# runnable locally). CONVENTION: build-args here must stay TEXTUALLY
# IDENTICAL to the push step's build-args — same cache scope + same args
# means the layers are reused and the image we smoke IS the image we push.
- name: Build image for smoke (load, no push)
uses: docker/build-push-action@v6
with:
context: .
platforms: linux/amd64
build-args: |
APP_VERSION=${{ steps.version.outputs.value }}
AI_AGENT_ROLES_CATALOG_URL=https://raw.githubusercontent.com/vvzvlad/gitmost/develop/agent-roles-catalog
load: true
push: false
tags: gitmost:smoke
cache-from: type=gha,scope=develop-amd64
# Boot-smoke the exact image against the job services (see the comment on
# `services:` above): health (startup migrator), auth/setup, client dist
# served, immutable + brotli asset headers. Fails the job (and therefore
# the push) on any miss.
- name: Smoke the built image
run: bash scripts/ci/image-smoke.sh gitmost:smoke
# The smoke script leaves the container running on failure precisely so
# the boot error (migration mismatch, stack trace) is diagnosable here.
- name: Dump smoke container log on failure
if: failure()
run: docker logs gitmost-smoke 2>&1 | tail -200 || true
- name: Build and push develop image
uses: docker/build-push-action@v6
with:
@@ -220,12 +157,6 @@ 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
+13 -41
View File
@@ -25,65 +25,37 @@ jobs:
# filename sorts BEFORE migrations already applied on the target branch (and
# thus in prod). The Kysely startup migrator rejects that as "corrupted
# migrations" and crash-loops the app on boot (incident #361). This gate fails
# the PR so the migration is renamed to a current timestamp before merge.
# Runs for pull_request (diff against the base branch) AND for push (#476
# retrospective: a DIRECT push to develop used to bypass this PR-only gate
# entirely — now the push is diffed against its `before` SHA; workflow_call
# from develop.yml inherits the caller's push event). workflow_dispatch has
# nothing to diff against and still skips the job.
# the PR so the migration is renamed to a current timestamp before merge. Only
# runs for pull_request events (needs a base branch to diff against).
migration-order:
if: github.event_name == 'pull_request' || github.event_name == 'push'
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- name: Checkout (full history for the base diff)
- name: Checkout (full history for the base-branch diff)
uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Added migrations must sort after the newest on the base
- name: Added migrations must sort after the newest on the base branch
env:
TARGET_BRANCH: ${{ github.base_ref }}
BEFORE_SHA: ${{ github.event.before }}
run: |
set -euo pipefail
MIG_DIR="apps/server/src/database/migrations"
if [ "${GITHUB_EVENT_NAME}" = "pull_request" ]; then
# checkout above already did fetch-depth:0 (full history). Fetch the base
# WITHOUT --depth (a shallow graft would truncate the base history and
# break the merge-base when the base has moved ahead of the PR merge —
# exactly the long-branch-vs-moving-base case this gate guards, #361).
git fetch --no-tags origin "$TARGET_BRANCH"
BASE="origin/${TARGET_BRANCH}"
else
# push event: compare against the pre-push tip of the branch.
if [ "$BEFORE_SHA" = "0000000000000000000000000000000000000000" ]; then
echo "::notice::branch creation push — nothing to compare"
exit 0
fi
if ! git cat-file -e "${BEFORE_SHA}^{commit}" 2>/dev/null; then
# The before-SHA is not in the clone (a force-push rewrote history).
# One recovery attempt — refresh every remote head (cheap: the
# checkout is already fetch-depth:0); a fetch failure aborts via
# `set -e`, which is fail-closed too.
git fetch --no-tags origin '+refs/heads/*:refs/remotes/origin/*'
fi
if ! git cat-file -e "${BEFORE_SHA}^{commit}" 2>/dev/null; then
# FAIL-CLOSED: without the before-SHA there is no base to prove the
# ordering against, and a gate whose job is to BLOCK must not guess.
echo "::error::force-push detected — verify migration order manually, then re-run via workflow_dispatch"
exit 1
fi
BASE="$BEFORE_SHA"
fi
newest_on_target=$(git ls-tree -r --name-only "$BASE" "$MIG_DIR" | sort | tail -1)
# checkout above already did fetch-depth:0 (full history). Fetch the base
# WITHOUT --depth (a shallow graft would truncate the base history and
# break the merge-base when the base has moved ahead of the PR merge —
# exactly the long-branch-vs-moving-base case this gate guards, #361).
git fetch --no-tags origin "$TARGET_BRANCH"
newest_on_target=$(git ls-tree -r --name-only "origin/${TARGET_BRANCH}" "$MIG_DIR" | sort | tail -1)
# NO `|| true`: a diff failure (e.g. an unresolved merge-base) must fail
# the job CLOSED — a gate whose job is to BLOCK must never pass on error.
# `set -e` above already aborts on a non-zero diff exit.
added=$(git diff --diff-filter=A --name-only "${BASE}...HEAD" -- "$MIG_DIR")
added=$(git diff --diff-filter=A --name-only "origin/${TARGET_BRANCH}...HEAD" -- "$MIG_DIR")
bad=0
for f in $added; do
if [[ "$f" < "$newest_on_target" || "$f" == "$newest_on_target" ]]; then
echo "::error::Migration $f sorts at or before the newest on the base ($newest_on_target) — rename it with a CURRENT timestamp before merge (do not change its contents). See incident #361."
echo "::error::Migration $f sorts at or before the newest on ${TARGET_BRANCH} ($newest_on_target) — rename it with a CURRENT timestamp before merge (do not change its contents). See incident #361."
bad=1
fi
done
-16
View File
@@ -251,22 +251,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
by physical key position and matched against the commands; genuine Cyrillic
search terms keep priority over remapped candidates, and short wrong-layout
prefixes match by command title. (#283, #285, #287)
- **Opt-in substring "lookup" search mode for agents.** `/api/search` gains an
additive, opt-in mode (guarded by a new `substring` flag) that matches literal
substrings of page titles and body text — so technical tokens the full-text
tokenizer mangles (`backup-srv.local`, `10.0.12.5`, `WB-MGE-30D86B`) are found
even when the FTS query is empty. It returns a location `path`, a windowed
`snippet` and a per-response relevance `score`, supports `titleOnly` and a
`parentPageId` subtree scope, and applies the page-level permission filter
before the limit. The web UI never sets `substring`, so its full-text search
behaviour is byte-for-byte unchanged. The leading-wildcard `LIKE` predicates
are backed by GIN trigram indexes on `LOWER(f_unaccent(title))` and
`LOWER(f_unaccent(text_content))` so lookups use a bitmap index scan instead of
a sequential scan. (#443)
- **MCP `search` tool returns richer, agent-oriented results.** The external MCP
`search` response shape changes for the agent surface: each hit now carries
`pageId` (renamed from `id`), plus `path`, `snippet` and `score`; the
UI-oriented `spaceId`, `rank` and `highlight` fields are dropped. (#443)
### Changed
@@ -121,20 +121,13 @@ export default function AiMcpServerForm({
async function handleSubmit(values: FormValues) {
const headers = resolveHeaders();
// An empty tag field means "no restriction" and must be sent as null —
// since #476 the server persists a literal `[]` as deny-all (zero tools),
// so an empty array from this form would silently disable every tool of
// the server. Deny-all remains expressible via the API, not via this form.
const toolAllowlist =
values.toolAllowlist.length === 0 ? null : values.toolAllowlist;
if (isEdit && server) {
const payload: IAiMcpServerUpdate = {
id: server.id,
name: values.name,
transport: values.transport,
url: values.url,
toolAllowlist,
toolAllowlist: values.toolAllowlist,
// Always sent: a blank value clears the stored guidance (server -> null).
instructions: values.instructions,
enabled: values.enabled,
@@ -147,7 +140,7 @@ export default function AiMcpServerForm({
name: values.name,
transport: values.transport,
url: values.url,
toolAllowlist,
toolAllowlist: values.toolAllowlist,
// Blank => server stores null (no guidance).
instructions: values.instructions,
enabled: values.enabled,
@@ -27,9 +27,7 @@ export interface IAiMcpServerCreate {
// Auth headers map (e.g. { Authorization: 'Bearer ...' }). Encrypted on save;
// never returned.
headers?: Record<string, string>;
// Omit/null => no restriction; `[]` is persisted verbatim and means
// deny-all (zero tools) since #476.
toolAllowlist?: string[] | null;
toolAllowlist?: string[];
// Admin-authored prompt guidance (#180). Blank => stored as null.
instructions?: string;
enabled?: boolean;
@@ -45,9 +43,7 @@ export interface IAiMcpServerUpdate {
transport?: McpTransport;
url?: string;
headers?: Record<string, string>;
// Absent => unchanged; null => no restriction; `[]` is persisted verbatim
// and means deny-all (zero tools) since #476.
toolAllowlist?: string[] | null;
toolAllowlist?: string[];
// Admin-authored prompt guidance (#180). Absent => unchanged; blank => cleared.
instructions?: string;
enabled?: boolean;
@@ -37,13 +37,10 @@ export class CreateMcpServerDto {
@IsObject()
headers?: Record<string, string>;
// Omit/null => no restriction; `[]` is persisted verbatim and means deny-all
// (zero tools) since #476. @IsOptional() skips validation for null as well,
// so an explicit null is accepted.
@IsOptional()
@IsArray()
@IsString({ each: true })
toolAllowlist?: string[] | null;
toolAllowlist?: string[];
// Admin-authored guidance ("how/when to use this server's tools") injected
// into the agent system prompt next to the tool descriptions (#180). Trusted,
@@ -38,13 +38,10 @@ export class UpdateMcpServerDto {
@IsObject()
headers?: Record<string, string>;
// Absent => unchanged; null => no restriction; `[]` is persisted verbatim
// and means deny-all (zero tools) since #476. @IsOptional() skips validation
// for null as well, so an explicit null is accepted.
@IsOptional()
@IsArray()
@IsString({ each: true })
toolAllowlist?: string[] | null;
toolAllowlist?: string[];
// Admin-authored prompt guidance (#180). Absent => unchanged; blank => cleared
// (stored as null by the repo). Capped to bound prompt/token size.
@@ -1,169 +0,0 @@
import { type Tool } from 'ai';
import { McpClientsService } from './mcp-clients.service';
/**
* Tool-allowlist filtering semantics on the merged external toolset (#476).
*
* COVERAGE CHOICE (documented per issue #476): the full corrupt-row chain
* (DB value -> repo normalizeRow -> toolsFor filter) is covered on TWO levels
* instead of one live-stub-MCP-server integration test:
* (a) apps/server/test/integration/ai-mcp-server-repo.int-spec.ts pins the
* repo read/write semantics against a real Postgres — `[]` round-trips
* as jsonb `[]`, a present-but-corrupt value fails CLOSED to `[]` with
* an error log;
* (b) THIS spec pins what the toolset builder does with the repo's output —
* null = unrestricted, `['alpha']` = only alpha, `[]` (including the
* corrupt-row fallback) = ZERO tools.
* Together they prove the end-to-end property "corrupt/empty allowlist can
* never widen to all tools" without a live stub HTTP MCP server.
*
* The drive path mirrors mcp-namespacing.spec.ts: stub the repo's listEnabled,
* spy the private `connect` to return a fake client, inspect the merged keys.
*/
function fakeTool(): Tool {
return { description: 'x', inputSchema: undefined } as unknown as Tool;
}
interface FakeServer {
id: string;
name: string;
transport: string;
url: string;
headersEnc: string | null;
toolAllowlist: string[] | null;
}
function server(
over: Partial<FakeServer> & { id: string; name: string },
): FakeServer {
return {
transport: 'http',
url: 'https://example.com/mcp',
headersEnc: null,
toolAllowlist: null,
...over,
};
}
/**
* Build a service whose repo returns `servers` and whose fake clients expose
* `rawTools` from tools(). Returns the merged tool keys produced by toolsFor.
*/
async function mergedKeysFor(
servers: FakeServer[],
rawTools: Record<string, Tool>,
): Promise<string[]> {
const repoStub = {
listEnabled: jest.fn().mockResolvedValue(servers),
};
const service = new McpClientsService(repoStub as never, {} as never);
jest
.spyOn(
service as unknown as { connect: (s: FakeServer) => unknown },
'connect',
)
.mockImplementation(() =>
Promise.resolve({
tools: () => Promise.resolve(rawTools),
close: () => Promise.resolve(),
}),
);
const toolset = await service.toolsFor('ws-1');
// Release the lease so the service does not hold the fake clients open.
await Promise.all(toolset.clients.map((c) => c.close()));
return Object.keys(toolset.tools);
}
describe('external MCP tool-allowlist filtering (via toolsFor, #476)', () => {
afterEach(() => jest.restoreAllMocks());
const RAW = () => ({
alpha: fakeTool(),
beta: fakeTool(),
gamma: fakeTool(),
});
it("['alpha'] lets ONLY alpha through", async () => {
const keys = await mergedKeysFor(
[server({ id: 'id-1', name: 'srv', toolAllowlist: ['alpha'] })],
RAW(),
);
expect(keys).toEqual(['srv_alpha']);
});
it('null (no restriction) lets every tool through', async () => {
const keys = await mergedKeysFor(
[server({ id: 'id-1', name: 'srv', toolAllowlist: null })],
RAW(),
);
expect(keys.sort()).toEqual(['srv_alpha', 'srv_beta', 'srv_gamma']);
});
it('[] (deny-all) yields ZERO tools — an empty array is authoritative, not falsy (#476)', async () => {
// This is the regression the #476 change guards: `[]` used to fall through
// the old `allow.length > 0` check and expose ALL tools. It must expose NONE.
const keys = await mergedKeysFor(
[server({ id: 'id-1', name: 'srv', toolAllowlist: [] })],
RAW(),
);
expect(keys).toEqual([]);
});
it('the corrupt-row fallback ([] from the repo) also yields ZERO tools (#476)', async () => {
// The repo turns a present-but-corrupt tool_allowlist into `[]` (fail-closed,
// see normalizeRow in ai-mcp-server.repo.ts + the int-spec); this pins that
// the toolset builder honours that fallback as deny-all rather than allow-all.
const corruptFallback: string[] = [];
const keys = await mergedKeysFor(
[server({ id: 'id-1', name: 'srv', toolAllowlist: corruptFallback })],
RAW(),
);
expect(keys).toEqual([]);
});
it('allowlisted names not exposed by the server are ignored (no phantom tools)', async () => {
const keys = await mergedKeysFor(
[
server({
id: 'id-1',
name: 'srv',
toolAllowlist: ['alpha', 'does-not-exist'],
}),
],
RAW(),
);
expect(keys).toEqual(['srv_alpha']);
});
it('a deny-all server contributes no prompt instructions (0 tools merged)', async () => {
const repoStub = {
listEnabled: jest.fn().mockResolvedValue([
{
...server({ id: 'id-1', name: 'srv', toolAllowlist: [] }),
instructions: 'use the tools wisely',
},
]),
};
const service = new McpClientsService(repoStub as never, {} as never);
jest
.spyOn(
service as unknown as { connect: (s: FakeServer) => unknown },
'connect',
)
.mockImplementation(() =>
Promise.resolve({
tools: () => Promise.resolve(RAW()),
close: () => Promise.resolve(),
}),
);
const toolset = await service.toolsFor('ws-1');
await Promise.all(toolset.clients.map((c) => c.close()));
expect(Object.keys(toolset.tools)).toEqual([]);
// mergeNamespaced reported 0 contributed tools, so no guidance is attached.
expect(toolset.instructions).toEqual([]);
});
});
@@ -285,13 +285,9 @@ export class McpClientsService {
try {
client = await this.connectWithTimeout(server, CONNECT_TIMEOUT_MS);
const raw = await withTimeout(client.tools(), CONNECT_TIMEOUT_MS);
// Allowlist semantics (#476): null/absent = no restriction (all tools);
// ANY array — including `[]` — is authoritative, so an EMPTY allowlist
// yields ZERO tools (deny-all). Do NOT add a `.length > 0` escape here:
// that read `[]` as falsy and silently widened deny-all to allow-all
// (the repo also fails corrupt rows closed to `[]` for the same reason).
const allow = server.toolAllowlist;
const picked = Array.isArray(allow) ? pick(raw, allow) : raw;
const picked =
Array.isArray(allow) && allow.length > 0 ? pick(raw, allow) : raw;
// Bound each tool's execute with a per-call total-timeout guard before
// merging, so a single chatty-but-stuck call is aborted after the cap.
const guarded = wrapToolsWithCallTimeout(picked, callTimeoutMs);
@@ -100,8 +100,7 @@ export class McpServersService {
transport: dto.transport,
url: dto.url,
headersEnc,
// undefined => unchanged; null => no restriction; `[]` is persisted
// verbatim and means deny-all (#476).
// undefined => unchanged; [] / value handled by repo (empty => null).
toolAllowlist: dto.toolAllowlist,
// undefined => unchanged; blank => cleared (null) by the repo.
instructions: dto.instructions,
@@ -59,8 +59,6 @@ function __assertClientCallContract(client: DocmostClientLike): void {
void client.getWorkspace();
void client.getSpaces();
void client.listPages(s, n, true);
void client.getTree(s, s, n);
void client.getPageContext(s);
void client.listSidebarPages(s, s);
void client.getOutline(s);
void client.getPageJson(s);
@@ -123,23 +121,6 @@ function __assertClientCallContract(client: DocmostClientLike): void {
void client.drawioGet(s, s, 'xml');
void client.drawioCreate(s, { position: 'append', anchorNodeId: s }, s, s, 'elk');
void client.drawioUpdate(s, s, s, s, 'elk');
// --- draw.io high-level semantic tools (#425 stage 3) ---
void client.drawioEditCells(s, s, [{ op: 'delete', cellId: s }], s);
void client.drawioFromGraph(
s,
{ position: 'append', anchorNodeId: s },
{ nodes: [{ id: s, label: s }] },
'LR',
s,
'full',
s,
);
void client.drawioFromMermaid(
s,
{ position: 'append', anchorNodeId: s },
s,
s,
);
// --- write (comment) ---
void client.createComment(s, s, 'inline', s, s, s);
void client.resolveComment(s, true);
@@ -23,8 +23,6 @@ type DocmostClientMethod =
| 'getWorkspace'
| 'getSpaces'
| 'listPages'
| 'getTree'
| 'getPageContext'
| 'listSidebarPages'
| 'getOutline'
| 'getPageJson'
@@ -71,10 +69,6 @@ type DocmostClientMethod =
| 'drawioGet'
| 'drawioCreate'
| 'drawioUpdate'
// --- draw.io high-level semantic tools (#425 stage 3) ---
| 'drawioEditCells'
| 'drawioFromGraph'
| 'drawioFromMermaid'
// --- write (comment) ---
| 'createComment'
| 'resolveComment';
@@ -27,12 +27,10 @@ import type { DocmostClientLike } from './docmost-client.loader';
*/
describe('tool tier metadata (#332)', () => {
it('core set is the documented 13 + searchInPage + insertFootnote + getTree + getPageContext (17, #443)', () => {
expect(CORE_TOOL_KEYS).toHaveLength(17);
it('core set is the documented 13 + searchInPage + insertFootnote (15)', () => {
expect(CORE_TOOL_KEYS).toHaveLength(15);
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,14 +39,12 @@ export interface ToolCatalogEntry {
/**
* CORE (always-active) in-app tool keys 13 frequent/tiny tools + `searchInPage`
* (#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).
* (#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).
*/
export const CORE_TOOL_KEYS = [
'searchPages',
@@ -68,11 +66,6 @@ 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. */
@@ -12,22 +12,3 @@ export class SearchResponseDto {
updatedAt: Date;
space: Partial<Space>;
}
// Response shape for the opt-in agent-lookup mode (#443, `substring: true`).
// Additive to the FTS response: carries the location (`path`), a windowed
// `snippet` around the first match and a per-response sort `score`. The MCP
// layer maps `id → pageId`; `slugId` is never exposed.
export class SearchLookupResponseDto {
id: string;
slugId: string;
title: string;
parentPageId: string | null;
// Ancestor titles from the space root down to the direct parent; [] for a
// root page.
path: string[];
// ~300–500 chars around the first match (or a leading text window / extended
// ts_headline fallback).
snippet: string;
// 0..1 float, meaningful ONLY for sorting within one response.
score: number;
}
@@ -30,31 +30,6 @@ export class SearchDTO {
@IsOptional()
@IsNumber()
offset?: number;
// --- Opt-in agent-lookup mode (#443). ------------------------------------
// These fields are ADDITIVE and default-off: a web client that sends none of
// them gets byte-identical FTS behaviour and result shape. They are only read
// by the substring/path/snippet code path in SearchService.searchPage.
//
// NOTE (standalone stdio vs stock upstream): stock upstream validates this DTO
// with `whitelist: true`, so an older server silently strips these unknown
// fields and the request degrades gracefully to the plain FTS behaviour.
// Enables the hybrid substring branch (title + text_content LIKE) merged with
// the existing FTS branch, plus tiered ranking, path and windowed snippet.
@IsOptional()
@IsBoolean()
substring?: boolean;
// Restrict the search to a page and all of its descendants (inclusive).
@IsOptional()
@IsString()
parentPageId?: string;
// Match titles only; do not scan text_content.
@IsOptional()
@IsBoolean()
titleOnly?: boolean;
}
export class SearchShareDTO extends SearchDTO {
@@ -60,12 +60,6 @@ export class SearchController {
}
}
// #443 graceful degradation: on EE/Typesense instances the request routes to
// the Typesense backend, which does NOT implement the opt-in agent-lookup
// mode. The `substring`/`parentPageId`/`titleOnly` fields are silently ignored
// and the response carries no `path`/`snippet`/`score` and no substring/tier
// ranking — it degrades to plain Typesense FTS. The native lookup mode below
// is Postgres-search-driver only.
if (this.environmentService.getSearchDriver() === 'typesense') {
return this.searchTypesense(searchDto, {
userId: user.id,
@@ -1,95 +0,0 @@
import {
computeLookupScore,
escapeLikePattern,
SearchLookupTier,
} from './search.service';
/**
* Pure-function coverage for the #443 agent-lookup helpers:
* - escapeLikePattern: LIKE-metacharacter escaping so `%`/`_`/`\` are literals
* (the acceptance-table requirement that a query of `%` or `_` does NOT match
* everything);
* - computeLookupScore: the tiered 0..1 ranking score, where a stronger tier
* always outranks a weaker one regardless of the in-tier secondary signal.
*
* The DB-touching branch (substring UNION FTS, path CTE, snippet window) is
* covered by the integration spec against the real schema.
*/
describe('escapeLikePattern', () => {
it('escapes the LIKE metacharacters % _ and \\', () => {
expect(escapeLikePattern('%')).toBe('\\%');
expect(escapeLikePattern('_')).toBe('\\_');
expect(escapeLikePattern('\\')).toBe('\\\\');
});
it('escapes the backslash FIRST so it does not double-escape %/_', () => {
// Input `\%` must become `\\` + `\%` = `\\\%`, not `\\%`.
expect(escapeLikePattern('\\%')).toBe('\\\\\\%');
});
it('leaves ordinary technical chars (. - / digits) untouched', () => {
expect(escapeLikePattern('backup-srv.local')).toBe('backup-srv.local');
expect(escapeLikePattern('10.0.12')).toBe('10.0.12');
expect(escapeLikePattern('WB-MGE-30D86B')).toBe('WB-MGE-30D86B');
expect(escapeLikePattern('a/b')).toBe('a/b');
});
it('escapes only the metacharacters in a mixed string', () => {
expect(escapeLikePattern('50%_off.zip')).toBe('50\\%\\_off.zip');
});
it('is null/undefined-safe', () => {
expect(escapeLikePattern(undefined as any)).toBe('');
expect(escapeLikePattern(null as any)).toBe('');
});
});
describe('computeLookupScore', () => {
it('keeps every score within (0, 1]', () => {
for (const tier of [
SearchLookupTier.TITLE_EXACT,
SearchLookupTier.TITLE_SUBSTRING,
SearchLookupTier.TEXT,
]) {
for (const secondary of [0, 0.001, 1, 100, 1e6]) {
const s = computeLookupScore({ tier, secondary });
expect(s).toBeGreaterThan(0);
expect(s).toBeLessThanOrEqual(1);
}
}
});
it('a stronger tier ALWAYS outranks a weaker tier, whatever the secondary', () => {
// Weak tier with a huge secondary must still lose to a strong tier with a
// tiny secondary — tiers dominate.
const strongLowSecondary = computeLookupScore({
tier: SearchLookupTier.TITLE_EXACT,
secondary: 0,
});
const weakHighSecondary = computeLookupScore({
tier: SearchLookupTier.TEXT,
secondary: 1e9,
});
expect(strongLowSecondary).toBeGreaterThan(weakHighSecondary);
});
it('within a tier a larger secondary sorts higher', () => {
const lo = computeLookupScore({
tier: SearchLookupTier.TEXT,
secondary: 0.1,
});
const hi = computeLookupScore({
tier: SearchLookupTier.TEXT,
secondary: 5,
});
expect(hi).toBeGreaterThan(lo);
});
it('treats a negative/absent secondary as 0', () => {
const zero = computeLookupScore({ tier: SearchLookupTier.TEXT, secondary: 0 });
expect(computeLookupScore({ tier: SearchLookupTier.TEXT })).toBe(zero);
expect(
computeLookupScore({ tier: SearchLookupTier.TEXT, secondary: -5 }),
).toBe(zero);
});
});
+2 -401
View File
@@ -1,9 +1,6 @@
import { Injectable } from '@nestjs/common';
import { SearchDTO, SearchSuggestionDTO } from './dto/search.dto';
import {
SearchLookupResponseDto,
SearchResponseDto,
} from './dto/search-response.dto';
import { SearchResponseDto } from './dto/search-response.dto';
import { InjectKysely } from 'nestjs-kysely';
import { KyselyDB } from '@docmost/db/types/kysely.types';
import { sql } from 'kysely';
@@ -37,53 +34,6 @@ export function buildTsQuery(raw: string): string {
return tsquery(cleaned + '*');
}
// Escape the LIKE metacharacters (`%`, `_`, `\`) in a raw user query so every
// character — including `.`, `-`, `_`, `%`, `/` — is matched LITERALLY by a
// `col LIKE '%' || q || '%'` predicate. Without this, a query of `%` or `_`
// would match every row (see the #443 acceptance table). The backslash is the
// escape char (Postgres LIKE default), so it must be escaped first.
export function escapeLikePattern(raw: string): string {
return (raw ?? '')
.replace(/\\/g, '\\\\')
.replace(/%/g, '\\%')
.replace(/_/g, '\\_');
}
// Ranking tiers for the agent-lookup mode (#443), highest first. A hit's tier
// is the strongest way it matched; ties inside a tier break on a secondary
// signal (FTS rank, or first-match position). The numeric `score` returned to
// the caller is derived from (tier, secondary) and is meaningful ONLY for
// ordering within a single response.
export enum SearchLookupTier {
// Title equals the query, case-insensitively.
TITLE_EXACT = 3,
// Query is a substring of the title.
TITLE_SUBSTRING = 2,
// Query matched in the text (substring or FTS).
TEXT = 1,
}
export interface RankableHit {
tier: SearchLookupTier;
// Secondary in-tier signal, higher = better (e.g. ts_rank, or a
// position-derived closeness score). Defaults to 0.
secondary?: number;
}
// Map (tier, secondary) → a 0..1 float used ONLY to sort one response.
//
// Formula: score = (tier + squash(secondary)) / (maxTier + 1), where
// squash(x) = x / (1 + x) maps any non-negative secondary into [0, 1)
// so a stronger tier ALWAYS outranks a weaker one regardless of the secondary
// value, and within a tier a larger secondary sorts higher. maxTier is the top
// enum value (TITLE_EXACT = 3), so the divisor keeps the result in (0, 1].
export function computeLookupScore(hit: RankableHit): number {
const maxTier = SearchLookupTier.TITLE_EXACT;
const secondary = Math.max(0, hit.secondary ?? 0);
const squashed = secondary / (1 + secondary);
return (hit.tier + squashed) / (maxTier + 1);
}
@Injectable()
export class SearchService {
constructor(
@@ -100,19 +50,12 @@ export class SearchService {
userId?: string;
workspaceId: string;
},
): Promise<{ items: SearchResponseDto[] | SearchLookupResponseDto[] }> {
): Promise<{ items: SearchResponseDto[] }> {
const { query } = searchParams;
if (query.length < 1) {
return { items: [] };
}
// Opt-in agent-lookup mode (#443). Guarded by the `substring` flag so the
// web-UI (which never sets it) keeps byte-identical FTS behaviour below.
if (searchParams.substring) {
return this.searchPageLookup(searchParams, opts);
}
const searchQuery = buildTsQuery(query);
let queryResults = this.db
@@ -232,348 +175,6 @@ export class SearchService {
return { items: searchResults };
}
/**
* Agent-lookup search (#443, opt-in via `SearchDTO.substring`).
*
* ADDITIVE to the FTS path: runs a substring branch (title + optionally
* text_content, LIKE with metacharacters escaped) MERGED with the existing
* FTS branch, so technical tokens that the `english` tokenizer mangles
* (`backup-srv.local`, `10.0.12.5`, `WB-MGE-30D86B`) are still found even
* when `buildTsQuery()` returns '' for a dotted/numeric query. Results carry a
* location (`path`), a windowed `snippet` and a per-response `score`.
*
* The whole method is only reached when `substring: true`; the web-UI never
* sets it, so its behaviour is unchanged.
*/
private async searchPageLookup(
searchParams: SearchDTO,
opts: { userId?: string; workspaceId: string },
): Promise<{ items: SearchLookupResponseDto[] }> {
const rawQuery = searchParams.query.trim();
if (!rawQuery) {
return { items: [] };
}
const limit = Math.min(Math.max(searchParams.limit || 10, 1), 50);
// Normalize the query the same way as the FTS / suggest path: f_unaccent +
// lower, done in SQL. `q` is the escaped LIKE pattern body (literal chars).
const likeBody = escapeLikePattern(rawQuery);
// Compare against `LOWER(f_unaccent(col))`; unaccent+lower the needle too.
const needle = sql<string>`LOWER(f_unaccent(${rawQuery}))`;
const likePattern = sql<string>`LOWER(f_unaccent(${'%' + likeBody + '%'}))`;
const tsQuery = buildTsQuery(rawQuery);
const hasTsQuery = tsQuery.length > 0;
// --- Resolve the space scope. ---------------------------------------------
// Mirrors searchPage: explicit spaceId, else the authenticated user's member
// spaces. The share path is not exposed to this opt-in mode.
let spaceIds: string[] = [];
if (searchParams.spaceId) {
spaceIds = [searchParams.spaceId];
} else if (opts.userId) {
spaceIds = await this.spaceMemberRepo.getUserSpaceIds(opts.userId);
} else {
return { items: [] };
}
if (spaceIds.length === 0) {
return { items: [] };
}
// --- Optional parentPageId subtree scope (inclusive). ---------------------
// Reuse the same recursive-descendants pattern used for share-scope.
let descendantIds: string[] | null = null;
if (searchParams.parentPageId) {
const descendants = await this.pageRepo.getPageAndDescendants(
searchParams.parentPageId,
{ includeContent: false },
);
descendantIds = descendants.map((p: any) => p.id);
if (descendantIds.length === 0) {
return { items: [] };
}
}
// --- Candidate query: substring (title + text) UNION FTS. -----------------
// We compute everything the ranker needs in SQL and pull only small columns
// (never the whole text_content) into Node:
// - titleExact / titleSub: tier signals
// - textMatchPos: 1-based position of the first text match (0 = none)
// - ftsRank: ts_rank for the FTS secondary signal (0 when no tsquery)
// - snippet: windowed ~500 chars around the first text match, or a leading
// text window (title-only hit), or an extended ts_headline fallback.
const N_BEFORE = 60; // chars of context before the first match
const SNIPPET_LEN = 500;
let candidates = this.db
.selectFrom('pages')
.select([
'pages.id as id',
'pages.slugId as slugId',
'pages.title as title',
'pages.parentPageId as parentPageId',
// Tier signals.
sql<boolean>`LOWER(f_unaccent(coalesce(pages.title, ''))) = ${needle}`.as(
'titleExact',
),
sql<boolean>`LOWER(f_unaccent(coalesce(pages.title, ''))) LIKE ${likePattern} ESCAPE '\\'`.as(
'titleSub',
),
// 1-based position of the first text match (0 = no text match).
sql<number>`strpos(LOWER(f_unaccent(coalesce(pages.text_content, ''))), ${needle})`.as(
'textMatchPos',
),
// FTS secondary signal (0 when the tsquery is empty).
hasTsQuery
? sql<number>`ts_rank(pages.tsv, to_tsquery('english', f_unaccent(${tsQuery})))`.as(
'ftsRank',
)
: sql<number>`0`.as('ftsRank'),
// Windowed snippet, computed entirely in SQL. Priority:
// 1. window around the first text match;
// 2. otherwise (titleOnly: no snippet; else) a leading window of the
// page text (title-only hit);
// 3. otherwise an extended ts_headline for pure-FTS hits.
//
// #443 snippet-position fix: the match position (`strpos`) is computed in
// the LOWER(f_unaccent(...)) space, but f_unaccent is NOT length-
// preserving (ß→ss, æ→ae, …→..., ½→ 1/2, full-width forms), so slicing
// the ORIGINAL text at that position was misaligned — a single expanding
// char before the match shifted the window (or ran it past end → empty).
// We now slice from the SAME LOWER(f_unaccent(...)) string so position
// and slice share one coordinate space. DELIBERATE trade-off: the snippet
// loses original case/diacritics — acceptable for an agent-facing snippet
// (position accuracy over original-glyph fidelity). The ts_headline branch
// matches over the ORIGINAL text itself, so it is unaffected and kept as-is.
searchParams.titleOnly
? sql<string>`''`.as('snippet')
: sql<string>`
coalesce(
case
when strpos(LOWER(f_unaccent(coalesce(pages.text_content, ''))), ${needle}) > 0
then substring(
LOWER(f_unaccent(coalesce(pages.text_content, '')))
from greatest(1, strpos(LOWER(f_unaccent(coalesce(pages.text_content, ''))), ${needle}) - ${N_BEFORE})
for ${SNIPPET_LEN}
)
when coalesce(pages.text_content, '') <> ''
then substring(LOWER(f_unaccent(pages.text_content)) from 1 for 300)
${
hasTsQuery
? sql`else ts_headline('english', coalesce(pages.text_content, ''), to_tsquery('english', f_unaccent(${tsQuery})), 'MinWords=25, MaxWords=40, MaxFragments=3')`
: sql``
}
end,
''
)
`.as('snippet'),
])
.where('pages.deletedAt', 'is', null)
.where('pages.spaceId', 'in', spaceIds);
if (descendantIds) {
candidates = candidates.where('pages.id', 'in', descendantIds);
}
// Match predicate: title substring OR (unless titleOnly) text substring OR
// (unless titleOnly) FTS. The substring branch runs even when the tsquery is
// empty — that is the dotted/numeric-token case the FTS path misses.
//
// #443 dead-index fix: these two LIKE predicates MUST match the GIN trgm
// index expressions EXACTLY for Postgres to use them. The indexes are on the
// coalesce-FREE expressions `LOWER(f_unaccent(title))` (#348's
// idx_pages_title_trgm) and `LOWER(f_unaccent(text_content))` (this PR's
// idx_pages_text_content_trgm). A `coalesce(col,'')` wrapper here would make
// the query expression differ from the index expression and force a Seq Scan
// on pages for every lookup. Dropping coalesce is SEMANTICALLY EQUIVALENT:
// `NULL LIKE '%q%'` is NULL (falsy), so a NULL title/text simply doesn't
// match — exactly as an empty string wouldn't match `%q%`.
candidates = candidates.where((eb) => {
const ors = [
eb(
sql`LOWER(f_unaccent(pages.title))`,
'like',
sql`${likePattern} ESCAPE '\\'`,
),
];
if (!searchParams.titleOnly) {
ors.push(
eb(
sql`LOWER(f_unaccent(pages.text_content))`,
'like',
sql`${likePattern} ESCAPE '\\'`,
),
);
if (hasTsQuery) {
ors.push(
sql<boolean>`pages.tsv @@ to_tsquery('english', f_unaccent(${tsQuery}))` as any,
);
}
}
return eb.or(ors);
});
// Pull a generous candidate set (before permission filtering + limit).
// Cap it so a pathological match set cannot blow up memory; 200 >> limit
// (max 50) leaves ample headroom for the post-permission truncation.
//
// #443 cap-ordering fix: the 200-cap MUST be deterministic and relevance-
// biased. Without an ORDER BY, Postgres returns an ARBITRARY 200 rows, so on
// a broad match set (common word / short substring) a strong TITLE_EXACT hit
// could be among the dropped rows while 200 low-tier TEXT hits fill the cap.
// We order by the SAME SQL tier proxies the Node ranker uses — title-exact,
// then title-substring, then fts-rank (nulls last), then earliest text-match
// position — so the cap keeps the strongest candidates. The Node-side final
// tier sort + slice(0, limit) below still runs and stays authoritative; this
// ORDER BY only decides WHICH candidates survive the 200-cap.
// NB: a BARE integer literal in ORDER BY is read by Postgres as an ordinal
// column position (`ORDER BY 0` → "position 0 is not in select list"), so the
// no-tsquery fallback is `0::float`, not `0`.
const ftsRankExpr = hasTsQuery
? sql`ts_rank(pages.tsv, to_tsquery('english', f_unaccent(${tsQuery})))`
: sql`0::float`;
const candidatesCapped = candidates
// Raw-SQL ORDER BY expressions: pass the full `<expr> <dir>` as ONE arg
// (the two-arg form treats a raw-SQL second arg as an ORDER BY position).
.orderBy(
sql`(LOWER(f_unaccent(coalesce(pages.title, ''))) = ${needle}) desc`,
)
.orderBy(
sql`(LOWER(f_unaccent(coalesce(pages.title, ''))) LIKE ${likePattern} ESCAPE '\\') desc`,
)
.orderBy(sql`${ftsRankExpr} desc nulls last`)
// Earlier text match first; strpos returns 0 for "no match", which would
// sort BEFORE a real (>=1) position under plain ASC, so push 0 to the end.
.orderBy(
sql`case when strpos(LOWER(f_unaccent(coalesce(pages.text_content, ''))), ${needle}) = 0 then 2147483647 else strpos(LOWER(f_unaccent(coalesce(pages.text_content, ''))), ${needle}) end asc`,
);
let rows: any[] = await candidatesCapped.limit(200).execute();
if (rows.length === 0) {
return { items: [] };
}
// --- Permissions BEFORE limit. --------------------------------------------
// Apply the existing page-level post-filter to the MERGED set, then rank and
// only THEN truncate to `limit` — never lose the permission filter.
if (opts.userId) {
const accessibleIds =
await this.pagePermissionRepo.filterAccessiblePageIds({
pageIds: rows.map((r) => r.id),
userId: opts.userId,
spaceId: searchParams.spaceId,
workspaceId: opts.workspaceId,
});
const accessibleSet = new Set(accessibleIds);
rows = rows.filter((r) => accessibleSet.has(r.id));
}
if (rows.length === 0) {
return { items: [] };
}
// --- Tiered ranking + dedup. ----------------------------------------------
// Rows are already unique by id (single pages scan), so no cross-branch
// dedup is needed here; the tier captures the strongest match reason.
const ranked = rows.map((r) => {
let tier: SearchLookupTier;
let secondary: number;
if (r.titleExact) {
tier = SearchLookupTier.TITLE_EXACT;
secondary = Number(r.ftsRank) || 0;
} else if (r.titleSub) {
tier = SearchLookupTier.TITLE_SUBSTRING;
secondary = Number(r.ftsRank) || 0;
} else {
tier = SearchLookupTier.TEXT;
// Prefer earlier text matches; map position → closeness in (0, 1].
const pos = Number(r.textMatchPos) || 0;
secondary =
pos > 0 ? 1 / (1 + (pos - 1) / 100) : Number(r.ftsRank) || 0;
}
return { row: r, tier, score: computeLookupScore({ tier, secondary }) };
});
ranked.sort((a, b) => b.score - a.score);
const top = ranked.slice(0, limit);
// --- Batch ancestor path (ONE recursive CTE, not N+1). --------------------
const pathById = await this.buildAncestorPaths(top.map((t) => t.row.id));
const items: SearchLookupResponseDto[] = top.map((t) => ({
id: t.row.id,
slugId: t.row.slugId,
title: t.row.title,
parentPageId: t.row.parentPageId ?? null,
path: pathById.get(t.row.id) ?? [],
snippet: (t.row.snippet ?? '')
.replace(/\r\n|\r|\n/g, ' ')
.replace(/\s+/g, ' ')
.trim(),
score: t.score,
}));
return { items };
}
/**
* Batch ancestor-titles helper (#443): ONE recursive CTE seeded with ALL hit
* ids, walking UP parentPageId. Returns a map hitId ancestor titles ordered
* root direct parent (the hit's own title is excluded). Root pages map to
* an empty array. Avoids the N+1 of a per-page breadcrumb call.
*/
private async buildAncestorPaths(
hitIds: string[],
): Promise<Map<string, string[]>> {
const result = new Map<string, string[]>();
if (hitIds.length === 0) return result;
// ancestry(hit_id, page_id, title, parent_page_id, depth): seed one row per
// hit at depth 0 (the hit itself), then walk to parents (increasing depth).
const rows = await this.db
.withRecursive('ancestry', (db) =>
db
.selectFrom('pages')
.select([
'pages.id as hitId',
'pages.id as pageId',
'pages.title as title',
'pages.parentPageId as parentPageId',
sql<number>`0`.as('depth'),
])
.where('pages.id', 'in', hitIds)
.unionAll((exp) =>
exp
.selectFrom('pages as p')
.innerJoin('ancestry as a', 'p.id', 'a.parentPageId')
.select([
'a.hitId as hitId',
'p.id as pageId',
'p.title as title',
'p.parentPageId as parentPageId',
sql<number>`a.depth + 1`.as('depth'),
]),
),
)
.selectFrom('ancestry')
.select(['hitId', 'title', 'depth'])
// depth 0 is the hit itself — excluded from the path.
.where('depth', '>', 0)
.orderBy('hitId')
// Larger depth = closer to the space root. Ordering DESC gives
// root → parent once collected.
.orderBy('depth', 'desc')
.execute();
for (const r of rows as any[]) {
const list = result.get(r.hitId) ?? [];
list.push(r.title);
result.set(r.hitId, list);
}
return result;
}
async searchSuggestions(
suggestion: SearchSuggestionDTO,
userId: string,
@@ -35,25 +35,4 @@ describe('jsonbBind', () => {
expect(out).not.toBeNull();
expect(out).toBeDefined();
});
// preserveEmpty (#476): opts a column OUT of the empty-to-null collapse so an
// empty container is persisted verbatim (e.g. `[]` = deny-all for
// tool_allowlist). null stays null regardless of the flag.
describe('preserveEmpty', () => {
it('returns a (non-null) bind for an empty array', () => {
const out = jsonbBind([], { preserveEmpty: true });
expect(out).not.toBeNull();
expect(out).toBeDefined();
});
it('returns a (non-null) bind for an empty object', () => {
const out = jsonbBind({}, { preserveEmpty: true });
expect(out).not.toBeNull();
expect(out).toBeDefined();
});
it('still returns null for null (null means null, flag or not)', () => {
expect(jsonbBind(null, { preserveEmpty: true })).toBeNull();
});
});
});
@@ -1,55 +0,0 @@
import { type Kysely, sql } from 'kysely';
/**
* #443 trigram indexes for the opt-in agent-lookup search mode.
*
* The lookup mode adds a substring branch that runs leading-wildcard
* `LOWER(f_unaccent(col)) LIKE '%q%'` predicates on pages.title and
* pages.text_content. A leading wildcard cannot use a b-tree index, so without a
* GIN trigram index each such predicate is a sequential scan.
*
* - TITLE: the lookup-mode title predicate is `LOWER(f_unaccent(title)) LIKE
* '%q%'` (coalesce-free, so it can use a functional index), which is IDENTICAL
* to the one added for /search/suggest (#348). #348's perf-indexes migration
* already created `idx_pages_title_trgm` on `(LOWER(f_unaccent(title)))
* gin_trgm_ops`, so the title predicate is already covered — we do NOT
* re-create that index here (it would be redundant).
*
* - TEXT_CONTENT: NEW. The substring branch scans text_content when the query
* is not titleOnly. text_content is the large column, so a GIN trigram index
* on it is the meaningful acceleration for the lookup mode. The lookup search
* is ALWAYS space-scoped (spaceId or the user's member spaces), so on small
* instances a per-space sequential scan is tolerable but the index turns the
* `%q%` text predicate into a Bitmap Index Scan and removes the only
* unbounded-per-space cost of the feature. We add it. The trade-off is disk +
* write amplification on page edits (GIN trigram indexes are larger and slower
* to update than b-trees); on the small instances this fork targets that cost
* is acceptable and the read win on agent lookups is the priority.
*
* DEPLOY-TIME LOCK WARNING: plain (non-CONCURRENT) CREATE INDEX Kysely runs
* each migration in a transaction, so CONCURRENTLY is impossible. The build takes
* a SHARE lock that BLOCKS writes on `pages` for its duration. The text_content
* GIN build is the slow one and can take minutes on a large tenant. For large
* installations, run this in a maintenance window or build the index out-of-band
* with CREATE INDEX CONCURRENTLY before deploying (then `IF NOT EXISTS` no-ops
* here). Small/typical tenants are unaffected.
*/
export async function up(db: Kysely<any>): Promise<void> {
// The title predicate is served by #348's idx_pages_title_trgm — see header.
// Only the text_content index is introduced here.
// text_content trigram index. Its expression is coalesce-free —
// `LOWER(f_unaccent(text_content))` — to EXACTLY match the coalesce-free
// lookup-mode text substring predicate in search.service.ts, so Postgres can
// use it (a `coalesce(...)` mismatch would silently fall back to a Seq Scan).
await sql`
CREATE INDEX IF NOT EXISTS idx_pages_text_content_trgm
ON pages USING gin ((LOWER(f_unaccent(text_content))) gin_trgm_ops)
`.execute(db);
}
export async function down(db: Kysely<any>): Promise<void> {
// Only drop the index this migration introduced. idx_pages_title_trgm is owned
// by the #348 perf-indexes migration, so leave it for that migration's down().
await sql`DROP INDEX IF EXISTS idx_pages_text_content_trgm`.execute(db);
}
@@ -78,9 +78,7 @@ export class AiMcpServerRepo {
headersEnc: values.headersEnc ?? null,
// jsonb column: the postgres driver would otherwise encode a JS array as
// a Postgres array literal. Bind the JSON text and cast it to jsonb.
// preserveEmpty (#476): `[]` is a real value here (deny-all), distinct
// from null ("no restriction") — it must round-trip as `[]`, not null.
toolAllowlist: jsonbBind(values.toolAllowlist, { preserveEmpty: true }),
toolAllowlist: jsonbBind(values.toolAllowlist),
// Plain text column: blank/whitespace-only guidance is stored as null.
instructions: blankToNull(values.instructions),
enabled: values.enabled ?? true,
@@ -113,10 +111,7 @@ export class AiMcpServerRepo {
if (patch.url !== undefined) set.url = patch.url;
if (patch.headersEnc !== undefined) set.headersEnc = patch.headersEnc;
if (patch.toolAllowlist !== undefined) {
// preserveEmpty (#476): see insert — `[]` (deny-all) must not become null.
set.toolAllowlist = jsonbBind(patch.toolAllowlist, {
preserveEmpty: true,
});
set.toolAllowlist = jsonbBind(patch.toolAllowlist);
}
if (patch.instructions !== undefined) {
// Blank/whitespace-only guidance clears the column (stored as null).
@@ -163,9 +158,7 @@ export function blankToNull(value: string | null | undefined): string | null {
* fix), so the driver hands back a string like `'["a","b"]'` rather than an
* array. Be tolerant: normalize a JSON string to its value, then accept it only
* if it is an array of strings; null / a non-array / unparseable value / an
* array with a non-string element all become null. NOTE: null here only means
* "could not parse" the null-vs-deny-all policy decision lives in
* normalizeRow (#476: present-but-corrupt fails CLOSED to `[]`).
* array with a non-string element all become null (unrestricted).
*/
export function parseToolAllowlist(value: unknown): string[] | null {
// Shape guard only; the legacy double-encoding self-heal lives in
@@ -180,20 +173,17 @@ export function parseToolAllowlist(value: unknown): string[] | null {
/**
* Normalize a DB row so `toolAllowlist` is always `string[] | null`.
*
* FAIL-CLOSED (#476): a stored value that is PRESENT but cannot be parsed into
* a string[] (corrupt JSON, a non-array, non-string elements) degrades to `[]`
* = deny-all, so a corrupted allowlist can never silently widen to "the agent
* gets ALL of the server's tools" (the old fail-open null). An error line is
* logged (server id only, never the contents) so the admin can repair the row.
* A column that is truly NULL/absent stays `null` = "no restriction".
* FAIL-OPEN logging: a stored value that is present but cannot be parsed into a
* string[] (corrupt JSON, a non-array, non-string elements) degrades to `null` =
* "no restriction", so the agent silently gets ALL of the server's tools. Log
* one line (server id only, never the contents) so that widening is not silent.
*/
function normalizeRow(row: AiMcpServer): AiMcpServer {
const parsed = parseToolAllowlist(row.toolAllowlist);
if (parsed === null && row.toolAllowlist != null) {
logger.error(
`Corrupt tool_allowlist for MCP server ${row.id}; failing closed (NO tools allowed) — re-save the server's allowlist to repair it`,
logger.warn(
`Corrupt tool_allowlist for MCP server ${row.id}; ignoring it (no tool restriction applied)`,
);
return { ...row, toolAllowlist: [] };
}
return { ...row, toolAllowlist: parsed };
}
+7 -19
View File
@@ -78,30 +78,18 @@ export function violatedConstraint(err: unknown): string | undefined {
* verbatim); `::jsonb` then parses it into a real array/object. Read-side
* parsers repair rows written the old buggy way without a migration.
*
* Returns `null` for null/undefined. By default it ALSO returns `null` for
* "empty" values (an empty array, or an object with no own enumerable keys)
* most callers treat empty as "clear/unset", so an empty config never
* round-trips as `[]`/`{}`.
*
* `preserveEmpty` (issue #476) opts a column OUT of that empty-to-null
* normalization so `[]`/`{}` are persisted as real jsonb values. Needed where
* empty and null mean DIFFERENT things: an empty `tool_allowlist` is
* deny-all ("zero tools allowed"), while null is "no restriction" collapsing
* `[]` to null silently widened deny-all to allow-all. Deliberately an opt-in
* flag, NOT a global change: the other jsonb callers (model_config, source)
* keep the empty-means-unset contract.
* Returns `null` for null/undefined and for "empty" values (an empty array, or
* an object with no own enumerable keys) callers treat empty as "clear/unset",
* so an empty allowlist/config never round-trips as `[]`/`{}`.
*/
export function jsonbBind<T>(
value: T | null | undefined,
opts?: { preserveEmpty?: boolean },
): RawBuilder<T> | null {
if (value === null || value === undefined) return null;
if (!opts?.preserveEmpty) {
if (Array.isArray(value)) {
if (value.length === 0) return null;
} else if (typeof value === 'object') {
if (Object.keys(value as object).length === 0) return null;
}
if (Array.isArray(value)) {
if (value.length === 0) return null;
} else if (typeof value === 'object') {
if (Object.keys(value as object).length === 0) return null;
}
return sql<T>`${JSON.stringify(value)}::text::jsonb`;
}
@@ -182,7 +182,6 @@ describe('AiChatService run-stream attach [integration]', () => {
{
isAiChatDeferredToolsEnabled: () => false,
isAiChatResumableStreamEnabled: () => true,
isAiChatFinalStepLockdownEnabled: () => false,
} as any,
registry,
);
@@ -500,7 +499,6 @@ 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, isAiChatFinalStepLockdownEnabled: () => false } as any,
{ isAiChatDeferredToolsEnabled: () => 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, isAiChatFinalStepLockdownEnabled: () => false } as any,
{ isAiChatDeferredToolsEnabled: () => true } as any,
);
}
@@ -1,6 +1,5 @@
import { Kysely, sql } from 'kysely';
import { randomUUID } from 'node:crypto';
import { Logger } from '@nestjs/common';
import { AiMcpServerRepo } from '@docmost/db/repos/ai-chat/ai-mcp-server.repo';
import { getTestDb, destroyTestDb, createWorkspace } from './db';
@@ -55,11 +54,7 @@ describe('AiMcpServerRepo tool_allowlist jsonb round-trip [integration]', () =>
expect(Array.isArray(found?.toolAllowlist)).toBe(true);
});
// #476 (deliberate behaviour change): an empty allowlist used to be
// normalized to SQL NULL, which downstream means "no restriction" — so an
// admin's deny-all `[]` silently became allow-all. It must now round-trip as
// a real jsonb `[]` (deny-all), distinct from NULL.
it('an empty allowlist round-trips as jsonb [] (deny-all), not null (#476)', async () => {
it('an empty allowlist is normalized to null (no restriction), not []', async () => {
const row = await repo.insert({
workspaceId: ws,
name: `srv-${randomUUID()}`,
@@ -67,27 +62,7 @@ describe('AiMcpServerRepo tool_allowlist jsonb round-trip [integration]', () =>
url: 'https://example.com/mcp',
toolAllowlist: [],
});
// The column holds a real (empty) jsonb ARRAY, not SQL NULL.
expect(await jsonbTypeof(row.id)).toBe('array');
expect((await repo.findById(row.id, ws))?.toolAllowlist).toEqual([]);
});
it('update to [] persists jsonb [] and update to null clears to SQL NULL (#476)', async () => {
const row = await repo.insert({
workspaceId: ws,
name: `srv-${randomUUID()}`,
transport: 'http',
url: 'https://example.com/mcp',
toolAllowlist: ['search'],
});
// Deny-all via update: [] must survive as a real jsonb array.
await repo.update(row.id, ws, { toolAllowlist: [] });
expect(await jsonbTypeof(row.id)).toBe('array');
expect((await repo.findById(row.id, ws))?.toolAllowlist).toEqual([]);
// Explicit clear (null) still means "no restriction" = SQL NULL.
await repo.update(row.id, ws, { toolAllowlist: null });
// The column is SQL NULL, so jsonb_typeof returns SQL NULL (JS null).
expect(await jsonbTypeof(row.id)).toBeNull();
expect((await repo.findById(row.id, ws))?.toolAllowlist).toBeNull();
});
@@ -117,60 +92,23 @@ describe('AiMcpServerRepo tool_allowlist jsonb round-trip [integration]', () =>
expect(healed?.toolAllowlist).toEqual(['alpha', 'beta']);
});
// #476 (deliberate behaviour change, replaces the old FAIL-OPEN pin): a
// present-but-corrupt tool_allowlist used to degrade to null ("no
// restriction"), silently handing the agent ALL of the server's tools. It
// must now FAIL CLOSED to `[]` (deny-all) and log an error.
it('FAIL-CLOSED: a present-but-corrupt tool_allowlist reads back as [] (deny-all) + error log (#476)', async () => {
const errorSpy = jest
.spyOn(Logger.prototype, 'error')
.mockImplementation(() => undefined);
try {
// The column is PRESENT but does not parse into a string[] — a jsonb
// string scalar holding unparseable text (a truncated legacy write).
const id = randomUUID();
await sql`
INSERT INTO ai_mcp_servers (id, workspace_id, name, transport, url, tool_allowlist)
VALUES (
${id}, ${ws}, ${`srv-${id}`}, 'http', 'https://example.com/mcp',
to_jsonb(${'{oops'}::text)
)
`.execute(db);
// Sanity: the column is present (a jsonb string scalar), not SQL NULL.
expect(await jsonbTypeof(id)).toBe('string');
// ...and the read degrades to [] (fail-closed deny-all), not null.
expect((await repo.findById(id, ws))?.toolAllowlist).toEqual([]);
// The narrowing is not silent: an error names the server id (never the
// corrupt contents).
expect(errorSpy).toHaveBeenCalledWith(
expect.stringContaining(`Corrupt tool_allowlist for MCP server ${id}`),
);
expect(
errorSpy.mock.calls.some((c) => String(c[0]).includes('{oops')),
).toBe(false);
} finally {
errorSpy.mockRestore();
}
});
it('FAIL-CLOSED: corrupt non-array JSON (an object) also reads back as [] (#476)', async () => {
const errorSpy = jest
.spyOn(Logger.prototype, 'error')
.mockImplementation(() => undefined);
try {
const id = randomUUID();
await sql`
INSERT INTO ai_mcp_servers (id, workspace_id, name, transport, url, tool_allowlist)
VALUES (
${id}, ${ws}, ${`srv-${id}`}, 'http', 'https://example.com/mcp',
to_jsonb(${'{"not":"an array"}'}::text)
)
`.execute(db);
expect(await jsonbTypeof(id)).toBe('string');
expect((await repo.findById(id, ws))?.toolAllowlist).toEqual([]);
} finally {
errorSpy.mockRestore();
}
it('FAIL-OPEN: a present-but-corrupt tool_allowlist reads back as null (no restriction)', async () => {
// #185 re-review pt 8: normalizeRow's fail-open branch — the column is
// PRESENT but does not parse into a string[] (here a jsonb string scalar
// holding non-array JSON). The read must degrade to `null` ("no restriction"),
// not crash. (A warn is logged with the server id; not asserted here.)
const id = randomUUID();
await sql`
INSERT INTO ai_mcp_servers (id, workspace_id, name, transport, url, tool_allowlist)
VALUES (
${id}, ${ws}, ${`srv-${id}`}, 'http', 'https://example.com/mcp',
to_jsonb(${'{"not":"an array"}'}::text)
)
`.execute(db);
// Sanity: the column is present (a jsonb string scalar), not SQL NULL.
expect(await jsonbTypeof(id)).toBe('string');
// ...yet the read degrades to null (fail-open).
expect((await repo.findById(id, ws))?.toolAllowlist).toBeNull();
});
});
@@ -1,123 +0,0 @@
import { randomUUID } from 'node:crypto';
import { Kysely, sql } from 'kysely';
import {
getTestDb,
destroyTestDb,
createWorkspace,
createSpace,
} from './db';
/**
* #443 dead-index guard EXPLAIN on the REAL DB.
*
* The lookup mode's substring predicates run a leading-wildcard
* `LOWER(f_unaccent(col)) LIKE '%q%'`. Those are only fast when Postgres uses
* the GIN trigram indexes:
* - idx_pages_title_trgm on (LOWER(f_unaccent(title))) [#348]
* - idx_pages_text_content_trgm on (LOWER(f_unaccent(text_content))) [#443]
*
* Postgres uses a functional index ONLY when the query expression matches the
* index expression EXACTLY. The original lookup query wrapped the columns in
* `coalesce(col,'')`, which differs from the coalesce-FREE index expression and
* silently forced a Seq Scan on pages for EVERY lookup (the MCP client always
* sends substring:true). This test locks that in.
*
* Discriminator: `SET enable_seqscan = off` asks the planner "CAN this predicate
* use the index at all?" which is exactly what the coalesce bug breaks. With
* seqscan disabled:
* - the coalesce-FREE (fixed) predicate plans a Bitmap Index Scan on the trgm
* index (no Seq Scan on pages);
* - the coalesce-WRAPPED (buggy) predicate cannot use the index and falls back
* to a Seq Scan on pages even though seqscan is disabled.
* We assert both to prove the fix and to keep the regression from silently
* returning.
*/
describe('SearchService agent-lookup EXPLAIN — trgm index is live [integration]', () => {
let db: Kysely<any>;
let workspaceId: string;
let spaceId: string;
async function insertPage(title: string, textContent: string): Promise<void> {
const id = randomUUID();
await db
.insertInto('pages')
.values({
id,
slugId: `slug-${id.slice(0, 12)}`,
title,
textContent,
spaceId,
workspaceId,
})
.execute();
}
// Run EXPLAIN (no ANALYZE — we only inspect the chosen plan) and return the
// concatenated plan text.
async function explain(query: string): Promise<string> {
const rows = await sql<{ 'QUERY PLAN': string }>`EXPLAIN ${sql.raw(query)}`.execute(
db,
);
return (rows.rows as any[]).map((r) => r['QUERY PLAN']).join('\n');
}
beforeAll(async () => {
db = getTestDb();
workspaceId = (await createWorkspace(db)).id;
spaceId = (await createSpace(db, workspaceId)).id;
// Seed enough rows that a trigram index is a plausible plan. The content is
// varied so the '%needle%' pattern is selective.
for (let i = 0; i < 200; i++) {
await insertPage(
`seed-title-${i}`,
`seed body content number ${i} lorem ipsum dolor sit amet ${i}`,
);
}
await insertPage('backup-srv.local', 'the needle-token-xyz lives here');
// Keep the trgm indexes' stats fresh so the planner costs them correctly.
await sql`ANALYZE pages`.execute(db);
});
afterAll(async () => {
await destroyTestDb();
});
// Force the planner to answer "can the index be used?" rather than "is it
// cheaper than a seq scan on this size?". Restored after each test.
beforeEach(async () => {
await sql`SET enable_seqscan = off`.execute(db);
});
afterEach(async () => {
await sql`RESET enable_seqscan`.execute(db);
});
it('title predicate (coalesce-FREE, as fixed) uses idx_pages_title_trgm, not a Seq Scan', async () => {
const plan = await explain(
`SELECT id FROM pages WHERE LOWER(f_unaccent(title)) LIKE '%srv.local%'`,
);
expect(plan).toContain('idx_pages_title_trgm');
expect(plan).not.toMatch(/Seq Scan on pages/i);
});
it('text_content predicate (coalesce-FREE, as fixed) uses idx_pages_text_content_trgm, not a Seq Scan', async () => {
const plan = await explain(
`SELECT id FROM pages WHERE LOWER(f_unaccent(text_content)) LIKE '%needle-token%'`,
);
expect(plan).toContain('idx_pages_text_content_trgm');
expect(plan).not.toMatch(/Seq Scan on pages/i);
});
// Negative control: the OLD coalesce-wrapped predicate must NOT be able to use
// the index — even with seqscan disabled it can only Seq Scan pages. If this
// ever stops seq-scanning, the coalesce/index expressions have re-aligned and
// the guard above is no longer meaningful.
it('coalesce-WRAPPED text predicate (the bug) cannot use the index — falls to Seq Scan', async () => {
const plan = await explain(
`SELECT id FROM pages WHERE LOWER(f_unaccent(coalesce(text_content,''))) LIKE '%needle-token%'`,
);
expect(plan).not.toContain('idx_pages_text_content_trgm');
expect(plan).toMatch(/Seq Scan on pages/i);
});
});
@@ -1,462 +0,0 @@
import { randomUUID } from 'node:crypto';
import { Kysely } from 'kysely';
import { SearchService } from 'src/core/search/search.service';
import { PageRepo } from '@docmost/db/repos/page/page.repo';
import {
getTestDb,
destroyTestDb,
createWorkspace,
createSpace,
} from './db';
/**
* #443 agent-lookup search mode, acceptance on the REAL DB schema.
*
* Exercises SearchService.searchPage(..., { substring: true }) against a
* migrated Postgres: substring matching of technical tokens the FTS tokenizer
* mangles (backup-srv.local, 10.0.12.5, WB-MGE-30D86B, "Теги: Docker"), the
* populated path + snippet, parentPageId subtree scoping, titleOnly, the empty
* result, LIKE-metacharacter escaping (`%`/`_` must NOT match everything), the
* permission post-filter applied BEFORE the limit, and the web-UI path staying
* on the legacy FTS shape when `substring` is absent.
*
* The tsv column is populated by the pages_tsvector_trigger on insert, so the
* FTS branch is exercised too.
*/
describe('SearchService agent-lookup mode [integration]', () => {
let db: Kysely<any>;
let service: SearchService;
let workspaceId: string;
let spaceId: string;
// Direct page insert (the shared createPage seeder omits text_content /
// parent_page_id, both of which this mode depends on). Returns the id.
async function insertPage(args: {
title: string;
textContent?: string;
parentPageId?: string | null;
spaceId?: string;
}): Promise<string> {
const id = randomUUID();
await db
.insertInto('pages')
.values({
id,
slugId: `slug-${id.slice(0, 12)}`,
title: args.title,
textContent: args.textContent ?? null,
parentPageId: args.parentPageId ?? null,
spaceId: args.spaceId ?? spaceId,
workspaceId,
})
.execute();
return id;
}
// Build a SearchService wired to the real DB + a real PageRepo (only its
// recursive-descendants method is used by this mode, and it needs only `db`),
// with lightweight stubs for the space-membership and permission repos so a
// test can drive scope + the permission post-filter explicitly.
function buildService(opts?: {
userSpaceIds?: string[];
// ids to KEEP after the permission post-filter; undefined = keep all.
accessibleIds?: string[];
}): SearchService {
const pageRepo = new PageRepo(db as any, null as any, null as any);
const spaceMemberRepo = {
getUserSpaceIds: async () => opts?.userSpaceIds ?? [spaceId],
};
const pagePermissionRepo = {
filterAccessiblePageIds: async ({ pageIds }: { pageIds: string[] }) =>
opts?.accessibleIds
? pageIds.filter((id) => opts.accessibleIds!.includes(id))
: pageIds,
};
return new SearchService(
db as any,
pageRepo as any,
{} as any, // shareRepo — unused by the lookup path
spaceMemberRepo as any,
pagePermissionRepo as any,
);
}
beforeAll(async () => {
db = getTestDb();
workspaceId = (await createWorkspace(db)).id;
spaceId = (await createSpace(db, workspaceId)).id;
service = buildService();
});
afterAll(async () => {
await destroyTestDb();
});
it('finds `backup-srv.local` by the fragment `srv.local`', async () => {
const pageId = await insertPage({
title: 'backup-srv.local',
textContent: 'A backup server node.',
});
const { items } = (await service.searchPage(
{ query: 'srv.local', spaceId, substring: true } as any,
{ workspaceId },
)) as any;
expect(items.map((i: any) => i.id)).toContain(pageId);
const hit = items.find((i: any) => i.id === pageId);
expect(hit.title).toBe('backup-srv.local');
// slugId must never be part of the server response shape.
expect('slugId' in hit).toBe(true); // server carries it; MCP strips it
});
it('finds a page whose TEXT contains `10.0.12.5` by the fragment `10.0.12` (empty-tsquery case)', async () => {
const pageId = await insertPage({
title: 'Server inventory',
textContent: 'The backup box lives at IP: 10.0.12.5. Debian 12, backups.',
});
const { items } = (await service.searchPage(
{ query: '10.0.12', spaceId, substring: true } as any,
{ workspaceId },
)) as any;
const hit = items.find((i: any) => i.id === pageId);
expect(hit).toBeDefined();
// The windowed snippet must include the matched text.
expect(hit.snippet).toContain('10.0.12.5');
});
it('finds `WB-MGE-30D86B` (alphanumeric token with dashes) by title', async () => {
const pageId = await insertPage({
title: 'WB-MGE-30D86B',
textContent: 'Device page.',
});
const { items } = (await service.searchPage(
{ query: 'WB-MGE-30D86B', spaceId, substring: true } as any,
{ workspaceId },
)) as any;
const hit = items.find((i: any) => i.id === pageId);
expect(hit).toBeDefined();
// Exact title match → top tier (TITLE_EXACT=3) → score in [0.75, 1].
expect(hit.score).toBeGreaterThanOrEqual(0.75);
// And it is the top-ranked hit of its own result set.
expect(items[0].id).toBe(pageId);
});
it('finds every page whose text literally contains `Теги: Docker`', async () => {
const a = await insertPage({
title: 'Container host A',
textContent: 'Some notes.\nТеги: Docker, compose\nmore.',
});
const b = await insertPage({
title: 'Container host B',
textContent: 'Prelude.\nТеги: Docker\nepilogue.',
});
const noise = await insertPage({
title: 'Unrelated',
textContent: 'Теги: Kubernetes',
});
const { items } = (await service.searchPage(
{ query: 'Теги: Docker', spaceId, substring: true, limit: 50 } as any,
{ workspaceId },
)) as any;
const ids = items.map((i: any) => i.id);
expect(ids).toContain(a);
expect(ids).toContain(b);
expect(ids).not.toContain(noise);
});
it('populates a non-empty `path` for a nested hit and `[]` for a root hit', async () => {
const root = await insertPage({ title: 'Infrastructure' });
const mid = await insertPage({ title: 'Datacenter A', parentPageId: root });
const leaf = await insertPage({
title: 'unique-nested-host',
parentPageId: mid,
});
const { items } = (await service.searchPage(
{ query: 'unique-nested-host', spaceId, substring: true } as any,
{ workspaceId },
)) as any;
const hit = items.find((i: any) => i.id === leaf);
expect(hit.path).toEqual(['Infrastructure', 'Datacenter A']);
const rootHits = (await service.searchPage(
{ query: 'Infrastructure', spaceId, substring: true } as any,
{ workspaceId },
)) as any;
const rootHit = rootHits.items.find((i: any) => i.id === root);
expect(rootHit.path).toEqual([]);
});
it('scopes to a subtree with parentPageId (cutting off sibling branches)', async () => {
const branchA = await insertPage({ title: 'BranchA-root' });
const inA = await insertPage({
title: 'scoped-target-xyz',
parentPageId: branchA,
});
const branchB = await insertPage({ title: 'BranchB-root' });
const inB = await insertPage({
title: 'scoped-target-xyz',
parentPageId: branchB,
});
const { items } = (await service.searchPage(
{
query: 'scoped-target-xyz',
spaceId,
substring: true,
parentPageId: branchA,
} as any,
{ workspaceId },
)) as any;
const ids = items.map((i: any) => i.id);
expect(ids).toContain(inA);
expect(ids).not.toContain(inB);
});
it('includes the parent page itself in the parentPageId subtree', async () => {
const parent = await insertPage({ title: 'self-included-parent' });
await insertPage({ title: 'child-of-self', parentPageId: parent });
const { items } = (await service.searchPage(
{
query: 'self-included-parent',
spaceId,
substring: true,
parentPageId: parent,
} as any,
{ workspaceId },
)) as any;
expect(items.map((i: any) => i.id)).toContain(parent);
});
it('titleOnly does NOT match on text_content', async () => {
const pageId = await insertPage({
title: 'Plain title',
textContent: 'body mentions the-secret-token here',
});
const withText = (await service.searchPage(
{ query: 'the-secret-token', spaceId, substring: true } as any,
{ workspaceId },
)) as any;
expect(withText.items.map((i: any) => i.id)).toContain(pageId);
const titleOnly = (await service.searchPage(
{
query: 'the-secret-token',
spaceId,
substring: true,
titleOnly: true,
} as any,
{ workspaceId },
)) as any;
expect(titleOnly.items.map((i: any) => i.id)).not.toContain(pageId);
});
// #443 Fix #1 regression: f_unaccent is NOT length-preserving, so an
// expanding char (ß→ss, …→...) BEFORE the match shifted the strpos position
// relative to the ORIGINAL text and the snippet slice ran past end → empty.
// The position and the slice now share the LOWER(f_unaccent(...)) space, so
// the window is aligned and always contains the matched (unaccented) token.
it('returns a populated snippet when an unaccent-EXPANDING char precedes the match', async () => {
// 300 × `ß` (each f_unaccent-expands to `ss`) before the needle. Under the
// old code strpos returned a position ~593 in the expanded space but the
// slice ran over the ORIGINAL (~360 char) text → empty snippet, match lost.
const prefix = 'ß'.repeat(300);
const pageId = await insertPage({
title: 'Expanding-unaccent page',
textContent: `${prefix} needle-token-xyz trailing.`,
});
const { items } = (await service.searchPage(
{ query: 'needle-token-xyz', spaceId, substring: true } as any,
{ workspaceId },
)) as any;
const hit = items.find((i: any) => i.id === pageId);
expect(hit).toBeDefined();
// Snippet must be non-empty AND contain the matched token (unaccented form).
expect(hit.snippet.length).toBeGreaterThan(0);
expect(hit.snippet).toContain('needle-token-xyz');
});
// #443 Fix #2 regression: >200 matching pages for a broad substring, with
// exactly ONE exact-title hit. Without an ORDER BY on the 200-cap the exact
// hit could be among the arbitrarily-dropped rows; the ORDER BY keeps the
// strongest candidates so it must survive the cap and rank at the top.
it('keeps an exact-title hit through the 200-cap on a >200-row match set', async () => {
const isoSpace = (await createSpace(db, workspaceId)).id;
const svc = buildService({ userSpaceIds: [isoSpace] });
// 250 low-tier TEXT hits: the shared substring `capword` appears only in the
// body, never the title, so each is a TEXT-tier match (weakest tier).
for (let i = 0; i < 250; i++) {
await insertPage({
title: `filler-page-${i}`,
textContent: `body contains capword here #${i}`,
spaceId: isoSpace,
});
}
// Exactly one EXACT-title hit for the same query token.
const exact = await insertPage({
title: 'capword',
textContent: 'unrelated body text',
spaceId: isoSpace,
});
const { items } = (await svc.searchPage(
{ query: 'capword', spaceId: isoSpace, substring: true, limit: 10 } as any,
{ workspaceId },
)) as any;
const ids = items.map((i: any) => i.id);
// The exact-title hit must survive the 200-cap and appear in the top `limit`.
expect(ids).toContain(exact);
// And, being TITLE_EXACT, it must be the single strongest hit.
expect(items[0].id).toBe(exact);
});
// #443 Fix #3: titleOnly matches only the title, so it must not leak the page
// body as the snippet (the old "first 300 chars of text_content" fallback).
it('titleOnly does NOT return a text-body snippet', async () => {
const pageId = await insertPage({
title: 'titleonly-snippet-page',
textContent: 'SECRET-BODY-CONTENT-NOT-IN-TITLE that must not leak.',
});
const { items } = (await service.searchPage(
{
query: 'titleonly-snippet-page',
spaceId,
substring: true,
titleOnly: true,
} as any,
{ workspaceId },
)) as any;
const hit = items.find((i: any) => i.id === pageId);
expect(hit).toBeDefined();
// The body text must not appear in the snippet; titleOnly → empty snippet.
expect(hit.snippet).not.toContain('SECRET-BODY-CONTENT-NOT-IN-TITLE');
expect(hit.snippet).toBe('');
});
it('returns [] (not an error) for a query that matches nothing', async () => {
const { items } = (await service.searchPage(
{
query: 'zzz-no-such-string-anywhere-42',
spaceId,
substring: true,
} as any,
{ workspaceId },
)) as any;
expect(items).toEqual([]);
});
it('a `%` query does NOT match everything (LIKE metacharacter escaped)', async () => {
// Fresh space so we can assert on total counts without cross-test noise.
const isoSpace = (await createSpace(db, workspaceId)).id;
const svc = buildService({ userSpaceIds: [isoSpace] });
await insertPage({ title: 'alpha', spaceId: isoSpace });
await insertPage({ title: 'beta', spaceId: isoSpace });
const literal = await insertPage({
title: '100%-coverage',
spaceId: isoSpace,
});
const { items } = (await svc.searchPage(
{ query: '%', spaceId: isoSpace, substring: true, limit: 50 } as any,
{ workspaceId },
)) as any;
const ids = items.map((i: any) => i.id);
// `%` is a literal → matches only the page that actually contains '%'.
expect(ids).toContain(literal);
expect(ids).not.toContain(
items.find((i: any) => i.title === 'alpha')?.id,
);
expect(items.length).toBe(1);
});
it('an `_` query does NOT match everything (LIKE metacharacter escaped)', async () => {
const isoSpace = (await createSpace(db, workspaceId)).id;
const svc = buildService({ userSpaceIds: [isoSpace] });
await insertPage({ title: 'gamma', spaceId: isoSpace });
const literal = await insertPage({
title: 'snake_case_name',
spaceId: isoSpace,
});
const { items } = (await svc.searchPage(
{ query: '_', spaceId: isoSpace, substring: true, limit: 50 } as any,
{ workspaceId },
)) as any;
const ids = items.map((i: any) => i.id);
expect(ids).toContain(literal);
expect(items.length).toBe(1);
});
it('applies the permission post-filter to the MERGED set BEFORE the limit', async () => {
const isoSpace = (await createSpace(db, workspaceId)).id;
const keep = await insertPage({
title: 'perm-visible-target',
spaceId: isoSpace,
});
const hidden = await insertPage({
title: 'perm-hidden-target',
spaceId: isoSpace,
});
// Authenticated (userId set) so the permission filter runs; only `keep` is
// accessible. limit 1 must NOT be able to select `hidden`.
const svc = buildService({
userSpaceIds: [isoSpace],
accessibleIds: [keep],
});
const { items } = (await svc.searchPage(
{
query: 'perm-',
spaceId: isoSpace,
substring: true,
limit: 1,
} as any,
{ userId: 'user-1', workspaceId },
)) as any;
const ids = items.map((i: any) => i.id);
expect(ids).toContain(keep);
expect(ids).not.toContain(hidden);
});
it('web-UI path (no `substring` flag) keeps the legacy FTS response shape', async () => {
await insertPage({
title: 'legacy shape page',
textContent: 'searchable legacyword content',
});
const { items } = (await service.searchPage(
{ query: 'legacyword', spaceId } as any,
{ userId: 'user-1', workspaceId },
)) as any;
// Legacy hits carry rank + highlight + space, and NO path/snippet/score.
const hit = items[0];
expect(hit).toBeDefined();
expect('rank' in hit).toBe(true);
expect('highlight' in hit).toBe(true);
expect('path' in hit).toBe(false);
expect('snippet' in hit).toBe(false);
expect('score' in hit).toBe(false);
});
});
-63
View File
@@ -1,63 +0,0 @@
{
"$comment": "Semantic palettes for drawioFromGraph (issue #425). DATA, not code: node `kind` -> fill/stroke slot, edge `kind` -> line-style props, per preset. The `default` node palette is the issue's base table; `dark` keeps the same hues on a dark canvas with lighter strokes/font; `colorblind-safe` maps every slot onto the Okabe-Ito qualitative palette (8 colours proven distinguishable for all common colour-vision deficiencies) so no two adjacent kinds collide. `fontColor`/`fillColor`/`strokeColor` are exact draw.io values. `edgeDefault` is the fallback line style; `group` is the (always-transparent) container stroke per preset.",
"presets": {
"default": {
"canvasDark": false,
"nodes": {
"service": { "fillColor": "#dae8fc", "strokeColor": "#6c8ebf", "fontColor": "#000000" },
"db": { "fillColor": "#d5e8d4", "strokeColor": "#82b366", "fontColor": "#000000" },
"queue": { "fillColor": "#fff2cc", "strokeColor": "#d6b656", "fontColor": "#000000" },
"gateway": { "fillColor": "#ffe6cc", "strokeColor": "#d79b00", "fontColor": "#000000" },
"error": { "fillColor": "#f8cecc", "strokeColor": "#b85450", "fontColor": "#000000" },
"external": { "fillColor": "#f5f5f5", "strokeColor": "#666666", "fontColor": "#333333" },
"security": { "fillColor": "#e1d5e7", "strokeColor": "#9673a6", "fontColor": "#000000" }
},
"edges": {
"sync": { "props": "" },
"async": { "props": "dashed=1;" },
"error": { "props": "dashed=1;strokeColor=#DD344C;" }
},
"edgeDefault": { "strokeColor": "#333333", "fontColor": "#333333" },
"group": { "strokeColor": "#666666", "fontColor": "#333333" }
},
"dark": {
"canvasDark": true,
"nodes": {
"service": { "fillColor": "#1a2a44", "strokeColor": "#7ea6e0", "fontColor": "#dae8fc" },
"db": { "fillColor": "#1f331e", "strokeColor": "#97d077", "fontColor": "#d5e8d4" },
"queue": { "fillColor": "#3a3218", "strokeColor": "#e5c15a", "fontColor": "#fff2cc" },
"gateway": { "fillColor": "#3a2812", "strokeColor": "#ffb570", "fontColor": "#ffe6cc" },
"error": { "fillColor": "#3a1c1b", "strokeColor": "#e08e8b", "fontColor": "#f8cecc" },
"external": { "fillColor": "#2b2b2b", "strokeColor": "#999999", "fontColor": "#e0e0e0" },
"security": { "fillColor": "#2c2338", "strokeColor": "#b39ddb", "fontColor": "#e1d5e7" }
},
"edges": {
"sync": { "props": "" },
"async": { "props": "dashed=1;" },
"error": { "props": "dashed=1;strokeColor=#ff6b6b;" }
},
"edgeDefault": { "strokeColor": "#cccccc", "fontColor": "#e0e0e0" },
"group": { "strokeColor": "#aaaaaa", "fontColor": "#e0e0e0" }
},
"colorblind-safe": {
"canvasDark": false,
"okabeIto": ["#000000", "#E69F00", "#56B4E9", "#009E73", "#F0E442", "#0072B2", "#D55E00", "#CC79A7"],
"nodes": {
"service": { "fillColor": "#D6E9F5", "strokeColor": "#0072B2", "fontColor": "#000000" },
"db": { "fillColor": "#D6EFE4", "strokeColor": "#009E73", "fontColor": "#000000" },
"queue": { "fillColor": "#FCF8CC", "strokeColor": "#F0E442", "fontColor": "#000000" },
"gateway": { "fillColor": "#FBEBD0", "strokeColor": "#E69F00", "fontColor": "#000000" },
"error": { "fillColor": "#F7DDCC", "strokeColor": "#D55E00", "fontColor": "#000000" },
"external": { "fillColor": "#EDEDED", "strokeColor": "#000000", "fontColor": "#000000" },
"security": { "fillColor": "#F3DEEB", "strokeColor": "#CC79A7", "fontColor": "#000000" }
},
"edges": {
"sync": { "props": "" },
"async": { "props": "dashed=1;" },
"error": { "props": "dashed=1;strokeColor=#D55E00;" }
},
"edgeDefault": { "strokeColor": "#000000", "fontColor": "#000000" },
"group": { "strokeColor": "#000000", "fontColor": "#000000" }
}
}
}
+9 -385
View File
@@ -64,14 +64,6 @@ import {
} from "./lib/drawio-xml.js";
import { renderDiagramShapes } from "./lib/drawio-preview.js";
import { applyElkLayout } from "./lib/drawio-layout.js";
import {
buildFromGraph,
type Graph,
type LayoutMode as GraphLayoutMode,
} from "./lib/drawio-graph.js";
import { applyCellOps, type CellOp } from "./lib/drawio-cell-ops.js";
import { mermaidToGraph } from "./lib/drawio-mermaid.js";
import { parseCells as parseDrawioCells } from "./lib/drawio-xml.js";
import {
applyTextEdits,
TextEdit,
@@ -819,17 +811,13 @@ export class DocmostClient {
* large instances, so a single bounded page of results is returned (default
* 50, max 100) via the `/pages/recent` feed.
*
* Tree (`tree` true): DEPRECATED prefer `getTree`, which shares this exact
* code path (a single `/pages/tree` request via `enumerateSpacePages` +
* `buildPageTree`) but returns the compact `{pageId, title, children?,
* hasChildren?}` shape and supports `rootPageId`/`maxDepth`. This tree mode is
* kept for backward compatibility; it REQUIRES `spaceId` (a page tree is
* Tree (`tree` true): the space's FULL page hierarchy as a nested tree (each
* node has a `children` array). This mode REQUIRES `spaceId` (a page tree is
* scoped to one space) and IGNORES `limit` the whole hierarchy is returned.
* It fetches the tree via `enumerateSpacePages`, which on the fork server
* resolves to a single `/pages/tree` request returning the whole
* permission-filtered flat page set (soft-deleted pages excluded
* server-side); the cursor-BFS in `enumerateSpacePages` is only a fallback for
* stock upstream servers that lack `/pages/tree`.
* server-side).
*/
async listPages(spaceId?: string, limit: number = 50, tree: boolean = false) {
await this.ensureAuthenticated();
@@ -853,107 +841,6 @@ export class DocmostClient {
return items.map((page: any) => filterPage(page));
}
/**
* Fetch a space's page hierarchy (or one subtree) as a nested tree in a SINGLE
* request the #443 `getTree` tool. Shares its whole code path with
* `listPages(tree:true)`: `enumerateSpacePages` issues one `POST /pages/tree`
* (with the cursor-BFS only as a fallback for stock upstream servers that lack
* the endpoint), then `buildPageTree` nests the flat, permission-filtered,
* position-ordered list. No second tree fetch, no per-node BFS.
*
* - `rootPageId` restrict to that page's subtree; the server seeds the CTE
* with the page itself, so the result is exactly ONE root (the page and its
* descendants). Omit it for the whole space.
* - `maxDepth` trim the response to that many levels (roots = depth 1) to
* save tokens; the server still returns everything in one request, the cut
* is applied in `buildPageTree` AFTER the full tree is built. A node whose
* children were cut carries `hasChildren: true` (source of truth = the flat
* item's server `hasChildren`) so the caller can descend with a follow-up
* `getTree(spaceId, rootPageId=that node)` call.
*
* Output nodes are `{pageId, title, children?, hasChildren?}` only the UUID
* `pageId` is exposed (never `slugId`/`icon`/`position`). Requires `spaceId`
* (a page tree is scoped to one space).
*/
async getTree(spaceId: string, rootPageId?: string, maxDepth?: number) {
await this.ensureAuthenticated();
if (!spaceId) {
throw new Error(
"getTree: spaceId is required (a page tree is scoped to one space).",
);
}
const { pages } = await this.enumerateSpacePages(spaceId, rootPageId);
return buildPageTree(pages, { shape: "getTree", maxDepth });
}
/**
* "Where am I / what's around" for a single page the #443 `getPageContext`
* tool. Metadata only (no page content), using exactly TWO server requests:
*
* 1. `POST /pages/breadcrumbs` a recursive CTE that walks UP from the page.
* The server returns the chain root->page order (it `.reverse()`s the
* child-first walk before responding), INCLUDING the page itself as the
* LAST element. So the last element is the page and everything before it
* is the ancestor chain root->parent. This carries the page's own title
* and spaceId, so no extra page-info fetch is needed for a UUID input.
* 2. `listSidebarPages(spaceId, pageId)` the page's DIRECT children,
* cursor-paginated (a page with >20 children returns ALL of them, no
* dupes) and in sidebar `position` order, each carrying `hasChildren`.
*
* The input may be a slugId (agents copy them from URLs); it is run through
* `resolvePageId` first, exactly like the other page tools. A UUID input adds
* no request there (short-circuit), keeping the total at two; a slugId input
* adds one unavoidable resolve round-trip.
*
* INVARIANT: only the UUID `pageId` is exposed anywhere server `id` is
* mapped to `pageId` and `slugId` is never leaked. A nonexistent/inaccessible
* pageId makes the server 404/403, which propagates as a clear tool error
* (never a hollow empty object).
*/
async getPageContext(pageId: string) {
await this.ensureAuthenticated();
// Resolve a possibly-slugId input to the canonical UUID (no round-trip for a
// UUID). Errors here (bad/inaccessible id) propagate as a clear tool error.
const pageUuid = await this.resolvePageId(pageId);
// Request 1: the ancestor chain, root->page, page included as the LAST item.
const response = await this.client.post("/pages/breadcrumbs", {
pageId: pageUuid,
});
const chain: any[] = (response.data?.data ?? response.data) ?? [];
if (!Array.isArray(chain) || chain.length === 0) {
// The endpoint always includes the page itself, so an empty chain means
// the page is gone/inaccessible — surface a clear error, not {}.
throw new Error(`getPageContext: page "${pageId}" not found or inaccessible`);
}
// Split: the last element is the page, the rest (root->parent) are the
// breadcrumbs. A root page has no ancestors -> breadcrumbs is [].
const self = chain[chain.length - 1];
const ancestors = chain.slice(0, -1);
const page = {
pageId: self.id,
title: self.title,
spaceId: self.spaceId,
};
const breadcrumbs = ancestors.map((n: any) => ({
pageId: n.id,
title: n.title,
}));
// Request 2: direct children in sidebar order, each with hasChildren.
const childItems = await this.listSidebarPages(self.spaceId, pageUuid);
const children = childItems.map((c: any) => ({
pageId: c.id,
title: c.title,
hasChildren: Boolean(c.hasChildren),
}));
return { page, breadcrumbs, children };
}
/**
* List sidebar pages for a space. With no pageId the request returns the
* space ROOT pages; with a pageId it returns the direct CHILDREN of that
@@ -2943,28 +2830,14 @@ export class DocmostClient {
return { success: true, removedShareId: share.shareId, pageId };
}
async search(
query: string,
spaceId?: string,
limit?: number,
opts: { parentPageId?: string; titleOnly?: boolean } = {},
) {
async search(query: string, spaceId?: string, limit?: number) {
await this.ensureAuthenticated();
// Opt into the #443 agent-lookup mode: `substring: true` turns on the hybrid
// substring + FTS branch that returns path + snippet + score. A stock
// upstream server strips these unknown DTO fields (whitelist:true) and
// silently degrades to plain FTS — see the tool-registration comment.
const payload: Record<string, any> = {
query,
spaceId,
substring: true,
};
if (opts.parentPageId) payload.parentPageId = opts.parentPageId;
if (opts.titleOnly) payload.titleOnly = true;
// Clamp an optional caller-supplied limit into the lookup range (1..50)
// before forwarding; omit it when not provided so the server default applies.
const payload: Record<string, any> = { query, spaceId };
// Clamp an optional caller-supplied limit into a sane 1..100 range before
// forwarding it to the server; omit it entirely when not provided so the
// server applies its own default.
if (limit !== undefined) {
payload.limit = Math.max(1, Math.min(50, limit));
payload.limit = Math.max(1, Math.min(100, limit));
}
const response = await this.client.post("/search", payload);
@@ -4638,255 +4511,6 @@ export class DocmostClient {
};
}
// --- draw.io high-level semantic tools (issue #425) ---
/**
* ID-based targeted edits of an existing drawio diagram (add / update / delete
* cells) instead of resending the whole XML. Reads the CURRENT diagram, checks
* the optimistic lock (`baseHash` is MANDATORY, exactly as drawioUpdate), applies
* the operations to the parsed model (a `delete` CASCADES to container children
* and to every edge whose source/target is deleted), then runs the SAME #423
* pipeline as drawioUpdate (lint + quality warnings -> preview -> attachment ->
* repoint the node). Ids are stable so diffs stay meaningful across edits.
*/
async drawioEditCells(
pageId: string,
node: string,
operations: CellOp[],
baseHash: string,
): Promise<{
success: boolean;
nodeId: string;
attachmentId: string;
warnings: string[];
verify?: any;
}> {
await this.ensureAuthenticated();
if (typeof baseHash !== "string" || baseHash.length === 0) {
throw new Error(
"drawioEditCells: baseHash is mandatory — read the diagram with drawioGet first and pass back its meta.hash",
);
}
if (!Array.isArray(operations) || operations.length === 0) {
throw new Error(
"drawioEditCells: operations must be a non-empty array of { op, ... }",
);
}
const { node: drawio, ref } = await this.resolveDrawioNode(pageId, node);
const oldAttrs = drawio.attrs || {};
const oldSrc = oldAttrs.src;
const nodeId = oldAttrs.id ?? ref;
if (!oldSrc) {
throw new Error(
`drawioEditCells: node "${node}" on page ${pageId} has no src to edit`,
);
}
const currentSvg = await this.fetchAttachmentText(oldSrc);
const currentModel = decodeDrawioSvg(currentSvg);
const currentHash = mxHash(currentModel);
if (currentHash !== baseHash) {
throw new Error(
`drawioEditCells: conflict — the diagram changed since it was read ` +
`(baseHash ${baseHash} != current ${currentHash}). Re-read it with drawioGet and retry.`,
);
}
// Apply the operations to the parsed model, then run the standard pipeline.
const editedModel = applyCellOps(currentModel, operations);
const prepared = prepareModel(editedModel);
const inner = renderDiagramShapes(prepared.cells, prepared.bbox);
const diagramTitle = oldAttrs.title || "Page-1";
const svg = buildDrawioSvg(prepared.modelXml, inner, prepared.bbox, diagramTitle);
const att = await this.uploadAttachmentBuffer(
pageId,
Buffer.from(svg, "utf-8"),
"diagram.drawio.svg",
"image/svg+xml",
);
const newSrc = `/api/files/${att.id}/${att.fileName}`;
const collabToken = await this.getCollabTokenWithReauth();
const pageUuid = await this.resolvePageId(pageId);
let repointed = 0;
const mutation = await this.mutatePage(
pageUuid,
collabToken,
this.apiUrl,
(liveDoc) => {
repointed = 0;
const doc =
liveDoc && liveDoc.type === "doc" ? liveDoc : { type: "doc", content: [] };
if (!Array.isArray(doc.content)) doc.content = [];
const hit = getNodeByRef(doc, ref);
if (!hit || hit.type !== "drawio") return null;
let target: any = doc;
for (const idx of hit.path) {
if (!target || !Array.isArray(target.content)) {
target = null;
break;
}
target = target.content[idx];
}
if (!target || target.type !== "drawio") return null;
target.attrs = {
...target.attrs,
src: newSrc,
attachmentId: att.id,
width: prepared.bbox.width,
height: prepared.bbox.height,
};
repointed++;
return doc;
},
);
if (repointed === 0) {
return {
success: true,
nodeId,
attachmentId: att.id,
warnings: [
...prepared.warnings,
"target drawio node was removed concurrently; uploaded attachment is unreferenced",
],
verify: mutation.verify,
};
}
return {
success: true,
nodeId,
attachmentId: att.id,
warnings: prepared.warnings,
verify: mutation.verify,
};
}
/**
* The main high-level tool: build a diagram from a SEMANTIC graph (nodes with
* a `kind`/`icon`, groups, edges) the model never supplies coordinates or
* style strings. The server resolves icons via the shape catalog (#424),
* assigns palette colors from the preset, runs ELK layered layout (honouring
* `direction` and the `layer`/`sameLayerAs`/`pinned` hints and compound groups),
* and assembles linter-clean XML, then inserts it through the SAME create
* pipeline as drawioCreate. `layout:"incremental"` is only meaningful when a
* target `node` is given (it preserves that diagram's existing coordinates and
* places only new cells); on a fresh insert it behaves like "full".
*/
async drawioFromGraph(
pageId: string,
where: {
position: "before" | "after" | "append";
anchorNodeId?: string;
anchorText?: string;
},
graph: Graph,
direction?: "LR" | "RL" | "TB" | "BT",
preset?: string,
layout?: GraphLayoutMode,
node?: string,
): Promise<{
success: boolean;
nodeId: string;
attachmentId: string;
warnings: string[];
iconsResolved: number;
iconsMissing: string[];
verify?: any;
}> {
await this.ensureAuthenticated();
// Direction/preset supplied as separate params override the graph fields so
// both the flat tool schema and an inline graph can set them.
const merged: Graph = {
...graph,
direction: direction ?? graph.direction,
preset: preset ?? graph.preset,
};
const mode: GraphLayoutMode = layout ?? "full";
// Incremental into an EXISTING node: read its coords so ELK preserves them,
// and keep the full existing model so incremental MERGES (never drops) any
// cell the new graph doesn't re-list.
let existingCoords: Map<string, { x: number; y: number }> | undefined;
let existingModelXml: string | undefined;
let editExisting = false;
let baseHash: string | undefined;
if (node && (mode === "incremental" || mode === "none")) {
const { node: drawio } = await this.resolveDrawioNode(pageId, node);
const src = (drawio.attrs || {}).src;
if (src) {
const svg = await this.fetchAttachmentText(src);
const model = decodeDrawioSvg(svg);
baseHash = mxHash(model);
existingModelXml = model;
existingCoords = new Map();
for (const c of parseDrawioCells(model)) {
if (c.vertex && c.geometry.x != null && c.geometry.y != null) {
existingCoords.set(c.id, { x: c.geometry.x, y: c.geometry.y });
}
}
editExisting = true;
}
}
const built = await buildFromGraph(
merged,
mode,
existingCoords,
existingModelXml,
);
if (editExisting && node && baseHash) {
// Re-target the existing diagram: replace it with the assembled model.
const res = await this.drawioUpdate(pageId, node, built.modelXml, baseHash);
return {
...res,
iconsResolved: built.iconsResolved,
iconsMissing: built.iconsMissing,
};
}
const res = await this.drawioCreate(pageId, where, built.modelXml);
return {
...res,
iconsResolved: built.iconsResolved,
iconsMissing: built.iconsMissing,
};
}
/**
* Convert a Mermaid `flowchart` to a redactable draw.io diagram via a PURE
* parser (no Electron / draw.io CLI): mermaid text -> graph-JSON -> the
* drawioFromGraph pipeline. Only `flowchart`/`graph` is supported (the most
* common wiki case); other diagram types throw a clear error so the model can
* fall back to drawioFromGraph.
*/
async drawioFromMermaid(
pageId: string,
where: {
position: "before" | "after" | "append";
anchorNodeId?: string;
anchorText?: string;
},
mermaid: string,
preset?: string,
): Promise<{
success: boolean;
nodeId: string;
attachmentId: string;
warnings: string[];
iconsResolved: number;
iconsMissing: string[];
verify?: any;
}> {
await this.ensureAuthenticated();
const graph = mermaidToGraph(mermaid);
if (preset) graph.preset = preset;
return this.drawioFromGraph(pageId, where, graph, graph.direction, graph.preset);
}
// --- Page history / diff / transform ---
/**
+11 -44
View File
@@ -435,63 +435,30 @@ server.registerTool(
// Tool: search
// INTENTIONAL per-transport divergence (not shared): the in-app `searchPages`
// runs a semantic + keyword hybrid (RRF) with in-process access control and a
// different schema; this transport is the #443 agent-lookup search — a hybrid
// substring + full-text search that also returns each hit's location (`path`)
// and a windowed `snippet`, so one call answers "where is it and what's in it".
// The in-app hybrid-RRF search is deliberately NOT touched. Different behaviour
// AND schema, so kept per-layer.
//
// STANDALONE-vs-STOCK-UPSTREAM: the client sends the opt-in `substring`/
// `parentPageId`/`titleOnly` DTO fields. A stock upstream server validates the
// DTO with `whitelist: true` and silently strips these unknown fields, so the
// request degrades gracefully to plain FTS (no path/snippet, current shape).
//
// EE/TYPESENSE DEGRADATION (#443): on an instance whose SEARCH_DRIVER is
// `typesense`, the server routes this request to the Typesense backend, which
// does NOT implement agent-lookup — the substring/path/snippet/tiering is
// ignored and the response degrades to plain Typesense FTS. The rich lookup
// shape is only produced by the native Postgres search driver.
// different schema (limit 1-20); this transport is a plain REST full-text search
// (limit up to 100). Different behaviour AND schema, so kept per-layer.
server.registerTool(
"search",
{
description:
"Find pages by a fragment of a technical string (hostnames, IPs, IDs " +
"like `srv.local`, `10.0.12`, `WB-MGE-30D86B`) — one call returns each " +
"hit's location (`path`: ancestor titles root→parent) and a `snippet` " +
"around the first match, so you rarely need a follow-up get_page. " +
"Matches substrings literally (dots/dashes/digits are not tokenized) as " +
"well as full-text. Returns `{ pageId, title, path, snippet, score }` " +
"sorted by `score` (a per-response relevance float).",
"Full-text search for pages and content across the whole workspace. " +
"Results are bounded by `limit` (1-100; when omitted the server applies " +
"its own default).",
inputSchema: {
query: z.string().min(1).describe("Search query"),
spaceId: z
.string()
.optional()
.describe("Restrict the search to a single space"),
parentPageId: z
.string()
.optional()
.describe(
"Restrict to a page and all its descendants (the page itself included)",
),
titleOnly: z
.boolean()
.optional()
.describe("Match page titles only; skip page text"),
limit: z
.number()
.int()
.min(1)
.max(50)
.max(100)
.optional()
.describe("Max results to return (1-50, default 10)"),
.describe("Max results to return (max 100)"),
},
},
async ({ query, spaceId, parentPageId, titleOnly, limit }) => {
const result = await docmostClient.search(query, spaceId, limit, {
parentPageId,
titleOnly,
});
async ({ query, limit }) => {
// The tool exposes no spaceId filter, so pass undefined for the client's
// optional spaceId parameter and forward limit into its correct slot.
const result = await docmostClient.search(query, undefined, limit);
return jsonContent(result);
},
);
+51 -161
View File
@@ -14,19 +14,7 @@
* signature.
*
* If recreateTransform / the changeset throws on a pathological document pair,
* 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).
* we fall back to a coarse block-level text diff so the tool never hard-fails.
*/
import { Node } from "@tiptap/pm/model";
@@ -84,56 +72,6 @@ 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"
@@ -288,81 +226,6 @@ 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">,
@@ -413,39 +276,66 @@ 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;
let tally: DiffTally;
const changedBlocks = new Set<string>();
// 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)) {
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.
fellBack = true;
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);
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);
}
}
const partial: Omit<DiffResult, "markdown"> = {
summary: {
inserted: tally.inserted,
deleted: tally.deleted,
blocksChanged: tally.changedBlocks.size,
},
summary: { inserted, deleted, blocksChanged: changedBlocks.size },
integrity,
changes: tally.changes,
changes,
};
return { ...partial, markdown: renderMarkdown(partial, fellBack) };
}
-168
View File
@@ -1,168 +0,0 @@
// ID-based cell operations for `drawioEditCells` (issue #425, stage 3).
//
// Instead of resending the whole XML (whose diff is fragile — draw.io reorders
// attributes, and a {search,replace} text match breaks on it), the model sends
// targeted operations keyed by cell id:
//
// { op: "add", xml: "<mxCell .../>" } // append a new cell
// { op: "update", cellId: "n3", xml: "<mxCell .../>" } // replace that cell
// { op: "delete", cellId: "n5" } // + CASCADE
//
// `delete` CASCADES: it removes the cell, every descendant cell whose parent
// chain leads to it (container children), AND every edge whose source or target
// is any deleted cell. Ids are STABLE across edits so diffs stay meaningful.
//
// Operations apply to the parsed DOM of the current model; the caller re-lints
// and rebuilds the .drawio.svg through the existing #423 pipeline afterwards.
import { JSDOM } from "jsdom";
let _window: any = null;
function xmlWindow(): any {
if (!_window) _window = new JSDOM("").window;
return _window;
}
export type CellOp =
| { op: "add"; xml: string }
| { op: "update"; cellId: string; xml: string }
| { op: "delete"; cellId: string };
export class CellOpsError extends Error {
constructor(message: string) {
super(`drawioEditCells: ${message}`);
this.name = "CellOpsError";
}
}
// The mxGraph root sentinels. id="0" is the graph root; id="1" is the default
// layer that parents every real cell. A delete targeting either would cascade
// through the whole diagram body (every cell chains up to "1"), so such an op is
// rejected outright.
const SENTINEL_IDS = new Set(["0", "1"]);
/** Parse a single `<mxCell …>…</mxCell>` fragment into an element, or throw. */
function parseCellFragment(xml: string): any {
const parser = new (xmlWindow().DOMParser)();
// Wrap so a self-closed or child-bearing single cell parses as one root.
const doc = parser.parseFromString(`<root>${xml}</root>`, "application/xml");
if (doc.getElementsByTagName("parsererror").length > 0) {
throw new CellOpsError(`operation xml is not well-formed: ${xml.slice(0, 120)}`);
}
const cells = doc.getElementsByTagName("mxCell");
if (cells.length !== 1) {
throw new CellOpsError(
`each add/update op must carry exactly one <mxCell> (got ${cells.length})`,
);
}
return cells[0];
}
/** All ids reachable as descendants of `rootId` via the parent relation. */
function collectDescendants(
rootId: string,
parentOf: Map<string, string | undefined>,
): Set<string> {
const doomed = new Set<string>([rootId]);
let grew = true;
while (grew) {
grew = false;
for (const [id, parent] of parentOf) {
if (!doomed.has(id) && parent != null && doomed.has(parent)) {
doomed.add(id);
grew = true;
}
}
}
return doomed;
}
/**
* Apply the operation list to a model XML string and return the new model XML.
* Uses the DOM so attribute order / formatting is preserved for untouched cells.
* Throws CellOpsError on an unknown target id or a malformed op fragment (so the
* model gets a precise error and nothing is half-applied).
*/
export function applyCellOps(modelXml: string, ops: CellOp[]): string {
if (!Array.isArray(ops) || ops.length === 0) {
throw new CellOpsError("operations must be a non-empty array");
}
const parser = new (xmlWindow().DOMParser)();
const doc = parser.parseFromString(modelXml, "application/xml");
if (doc.getElementsByTagName("parsererror").length > 0) {
throw new CellOpsError("the current diagram XML is not well-formed");
}
const root = doc.getElementsByTagName("root")[0];
if (!root) throw new CellOpsError("the current diagram has no <root> element");
const cellEls = () => Array.from(root.getElementsByTagName("mxCell")) as any[];
const byId = () => {
const m = new Map<string, any>();
for (const el of cellEls()) m.set(el.getAttribute("id") ?? "", el);
return m;
};
for (const op of ops) {
if (op.op === "add") {
const frag = parseCellFragment(op.xml);
const id = frag.getAttribute("id");
if (!id) throw new CellOpsError("an add op's <mxCell> is missing an id");
if (byId().has(id))
throw new CellOpsError(`add op id "${id}" already exists (use update)`);
root.appendChild(doc.importNode(frag, true));
} else if (op.op === "update") {
const map = byId();
const target = map.get(op.cellId);
if (!target)
throw new CellOpsError(`update target cell "${op.cellId}" does not exist`);
const frag = parseCellFragment(op.xml);
const newId = frag.getAttribute("id");
if (newId && newId !== op.cellId)
throw new CellOpsError(
`update op cellId "${op.cellId}" != the <mxCell> id "${newId}" (ids are stable)`,
);
// Replace the element in place so surrounding cells are untouched.
const imported = doc.importNode(frag, true);
target.parentNode.replaceChild(imported, target);
} else if (op.op === "delete") {
// Reject a sentinel-targeted delete BEFORE collecting descendants: "0"/"1"
// parent the entire diagram, so a cascade from either would wipe the whole
// model body (doomed.delete("0"/"1") only spared the sentinel itself, not
// its children).
if (SENTINEL_IDS.has(op.cellId))
throw new CellOpsError(
`cannot delete sentinel cell "${op.cellId}" (the graph root/default layer)`,
);
const map = byId();
if (!map.has(op.cellId))
throw new CellOpsError(`delete target cell "${op.cellId}" does not exist`);
// Build the parent relation over the CURRENT cells for the cascade.
const parentOf = new Map<string, string | undefined>();
for (const el of cellEls()) {
parentOf.set(el.getAttribute("id") ?? "", el.getAttribute("parent") ?? undefined);
}
const doomed = collectDescendants(op.cellId, parentOf);
// Cascade to edges whose source/target is any doomed cell.
for (const el of cellEls()) {
if (el.getAttribute("edge") !== "1") continue;
const src = el.getAttribute("source");
const tgt = el.getAttribute("target");
if ((src && doomed.has(src)) || (tgt && doomed.has(tgt))) {
doomed.add(el.getAttribute("id") ?? "");
}
}
// Never delete the sentinels even if referenced by a malformed op.
doomed.delete("0");
doomed.delete("1");
for (const el of cellEls()) {
const id = el.getAttribute("id") ?? "";
if (doomed.has(id)) el.parentNode.removeChild(el);
}
} else {
throw new CellOpsError(`unknown op "${(op as any).op}"`);
}
}
const ser = new (xmlWindow().XMLSerializer)();
return ser.serializeToString(doc.documentElement);
}
-916
View File
@@ -1,916 +0,0 @@
// Semantic graph -> draw.io pipeline for `drawioFromGraph` (issue #425, stage 3).
//
// The model describes a diagram SEMANTICALLY — nodes with a `kind` and an
// optional `icon`, groups (containers), edges with a `kind` — and NEVER sees a
// coordinate or a style string. This module owns the whole server-side pipeline:
//
// 1. validateGraph — a hand-written validator (no zod dependency, so this
// lib stays importable by client.ts without coupling to
// a zod major) that rejects malformed graphs early.
// 2. resolveNodeStyle — `icon` -> exact style via the shape catalog (#424);
// an UNKNOWN icon degrades to a generic shape by `kind`
// WITH the label (never an empty square). `kind` -> the
// preset palette slot.
// 3. graphToElk — graph -> ELK-JSON, honouring the layout hints
// (`layer`/`sameLayerAs` -> layer constraints, `pinned`
// -> a fixed node) and compound group nodes.
// 4. assembleModel — graph + ELK coordinates -> a full mxGraphModel XML
// that satisfies the #423 linter BY CONSTRUCTION
// (sentinels, transparent containers, relative child
// coords, cross-container edges parent="1", >=150px
// gaps from ELK spacing, escaped labels).
//
// The `layout` mode: "full" re-lays everything; "incremental" fixes existing
// coordinates (ELK interactive mode) and places only new nodes; "none" keeps the
// caller-provided/prior coordinates untouched.
import ELK from "elkjs/lib/elk.bundled.js";
import { JSDOM } from "jsdom";
import {
searchShapes,
awsServiceStyle,
type ShapeResult,
} from "./drawio-shapes.js";
import {
getPreset,
genericNodeStyle,
iconNodeStyle,
edgeStyle,
groupStyle,
type PresetData,
} from "./drawio-presets.js";
import { MIN_SHAPE_GAP } from "./drawio-xml.js";
// --- graph schema (plain TS + a hand validator) ----------------------------
export interface GraphNode {
id: string;
label: string;
kind?: string;
/** Icon reference, e.g. "aws:lambda" | "azure:cosmos" | "lambda". */
icon?: string;
/** Group (container) id this node belongs to. */
group?: string;
/** Layer hint (ELK layerChoiceConstraint): 0-based column/row index. */
layer?: number;
/** Put this node in the same layer as another node id. */
sameLayerAs?: string;
/** Fix this node at exact coordinates (an ELK fixed node). */
pinned?: { x: number; y: number };
}
export interface GraphGroup {
id: string;
label: string;
kind?: string;
/** Parent group id — lets a group nest inside another group (e.g. subnet in VPC). */
group?: string;
}
export interface GraphEdge {
from: string;
to: string;
label?: string;
kind?: string;
}
export interface Graph {
nodes: GraphNode[];
groups?: GraphGroup[];
edges?: GraphEdge[];
direction?: "LR" | "RL" | "TB" | "BT";
preset?: string;
}
export type LayoutMode = "none" | "full" | "incremental";
/** A structured validation error (mirrors the drawio linter's shape loosely). */
export class GraphValidationError extends Error {
issues: string[];
constructor(issues: string[]) {
super(`drawioFromGraph: invalid graph — ${issues.join("; ")}`);
this.name = "GraphValidationError";
this.issues = issues;
}
}
const MAX_GRAPH_NODES = 500; // parity with drawio-layout's ELK_MAX_NODES.
// Edge/group caps mirror drawio-layout's ELK_MAX_EDGES. Without an edge cap a
// tiny node set with a huge edge list (e.g. 500 nodes / 200000 edges) passes
// node validation, then graphToElk/runElk exhausts the heap SYNCHRONOUSLY inside
// elk.bundled.js — before the 5s ELK timeout can fire and OUTSIDE it entirely
// for the mapper/assembler — crashing the worker on LLM-authored input. Reject
// the over-limit shape here, before any layout or assembly runs.
export const MAX_GRAPH_EDGES = 1000; // parity with drawio-layout's ELK_MAX_EDGES.
export const MAX_GRAPH_GROUPS = 500; // groups are compound ELK nodes; bound them too.
/**
* Validate the graph structure BEFORE any layout/assembly so the model gets a
* precise, actionable error instead of a corrupt diagram. Throws
* GraphValidationError listing every problem.
*/
export function validateGraph(graph: Graph): void {
const issues: string[] = [];
if (!graph || typeof graph !== "object") {
throw new GraphValidationError(["graph must be an object"]);
}
if (!Array.isArray(graph.nodes) || graph.nodes.length === 0) {
throw new GraphValidationError(["graph.nodes must be a non-empty array"]);
}
// Size caps FIRST (fail fast, before touching per-element loops) so an
// over-limit graph can never reach the layout engine and OOM the worker.
if (graph.nodes.length > MAX_GRAPH_NODES) {
throw new GraphValidationError([
`graph has ${graph.nodes.length} nodes (max ${MAX_GRAPH_NODES})`,
]);
}
if (Array.isArray(graph.edges) && graph.edges.length > MAX_GRAPH_EDGES) {
throw new GraphValidationError([
`graph has ${graph.edges.length} edges (max ${MAX_GRAPH_EDGES})`,
]);
}
if (Array.isArray(graph.groups) && graph.groups.length > MAX_GRAPH_GROUPS) {
throw new GraphValidationError([
`graph has ${graph.groups.length} groups (max ${MAX_GRAPH_GROUPS})`,
]);
}
const nodeIds = new Set<string>();
const groupIds = new Set<string>();
for (const g of graph.groups ?? []) {
if (!g.id) issues.push("a group is missing its id");
else if (groupIds.has(g.id)) issues.push(`duplicate group id "${g.id}"`);
groupIds.add(g.id);
}
for (const n of graph.nodes) {
if (!n.id) issues.push("a node is missing its id");
else if (nodeIds.has(n.id)) issues.push(`duplicate node id "${n.id}"`);
else if (groupIds.has(n.id))
issues.push(`node id "${n.id}" collides with a group id`);
nodeIds.add(n.id);
if (typeof n.label !== "string" || n.label === "")
issues.push(`node "${n.id}" is missing a label`);
if (n.group != null && !groupIds.has(n.group))
issues.push(`node "${n.id}" references unknown group "${n.group}"`);
if (n.pinned != null) {
if (
typeof n.pinned.x !== "number" ||
typeof n.pinned.y !== "number" ||
!Number.isFinite(n.pinned.x) ||
!Number.isFinite(n.pinned.y)
)
issues.push(`node "${n.id}" has an invalid pinned {x,y}`);
}
if (n.layer != null && (!Number.isInteger(n.layer) || n.layer < 0))
issues.push(`node "${n.id}" has an invalid layer (must be a >=0 integer)`);
}
// sameLayerAs must reference an existing node (checked after all ids known).
for (const n of graph.nodes) {
if (n.sameLayerAs != null && !nodeIds.has(n.sameLayerAs))
issues.push(
`node "${n.id}" sameLayerAs references unknown node "${n.sameLayerAs}"`,
);
}
for (const e of graph.edges ?? []) {
if (!e.from || !e.to) {
issues.push("an edge is missing from/to");
continue;
}
if (!nodeIds.has(e.from) && !groupIds.has(e.from))
issues.push(`edge from "${e.from}" resolves to no node/group`);
if (!nodeIds.has(e.to) && !groupIds.has(e.to))
issues.push(`edge to "${e.to}" resolves to no node/group`);
}
if (issues.length > 0) throw new GraphValidationError(issues);
}
// --- icon resolution -------------------------------------------------------
/**
* Resolve a node's `icon` reference to a concrete style-string + size via the
* shape catalog. Accepts "aws:lambda", "azure:cosmos", or a bare "lambda". An
* AWS `resIcon` name is built directly (exact service-icon template). Anything
* else goes through searchShapes. Returns null when nothing resolves the
* caller then falls back to a generic shape by kind (never an empty box).
*/
export function resolveIcon(icon: string): ShapeResult | null {
const raw = icon.trim();
if (raw === "") return null;
let provider = "";
let name = raw;
const colon = raw.indexOf(":");
if (colon !== -1) {
provider = raw.slice(0, colon).trim().toLowerCase();
name = raw.slice(colon + 1).trim();
}
if (provider === "aws") {
// Prefer an exact resIcon match from the catalog (carries the right size and
// any rebrand/blocklist note); if the underscore/space name doesn't hit,
// build the canonical service-icon style directly so it is never an empty box.
const results = searchShapes(name.replace(/_/g, " "), { limit: 5 });
const aws4 = results.find((r) => r.style.includes("mxgraph.aws4"));
if (aws4) return aws4;
return {
style: awsServiceStyle(name.replace(/\s+/g, "_")),
w: 78,
h: 78,
title: name,
type: "vertex",
};
}
// Non-AWS or bare name: fuzzy search the catalog. Take the top vertex hit,
// but REJECT a weak match (the fuzzy scorer can prefix-match an unrelated
// stencil, e.g. "not..." -> "Notebook"); require the hit's title to actually
// share a meaningful token with the query, otherwise degrade to generic-by-kind.
const q = provider ? `${provider} ${name}` : name;
const results = searchShapes(q, { limit: 8 });
const hit = results.find((r) => r.type !== "edge") ?? results[0];
if (!hit) return null;
if (!isRelevantMatch(name, hit.title)) return null;
return hit;
}
/**
* Whether a resolved stencil is a genuine match for the requested icon name (as
* opposed to a loose prefix hit on an unrelated shape). True if any 3+ char
* token of the query appears in the stencil title, or vice-versa.
*/
function isRelevantMatch(name: string, title: string): boolean {
const norm = (s: string) => s.toLowerCase().replace(/[^a-z0-9]+/g, " ").trim();
const qTokens = norm(name).split(/\s+/).filter((t) => t.length >= 3);
if (qTokens.length === 0) return true; // very short names: trust the scorer
const t = norm(title);
const tTokens = new Set(t.split(/\s+/));
for (const qt of qTokens) {
if (tTokens.has(qt)) return true;
if (t.includes(qt)) return true;
for (const tt of tTokens) if (tt.length >= 3 && qt.includes(tt)) return true;
}
return false;
}
/**
* Decide the final style + size for a node. When `icon` resolves, use the icon
* style (overlaid with a dark-preset font fix); otherwise a GENERIC shape by
* `kind` carrying the label. `resolved` reports whether an icon was found (used
* by the acceptance test that asserts no empty squares).
*/
export function resolveNodeStyle(
preset: PresetData,
node: GraphNode,
): { style: string; w: number; h: number; iconResolved: boolean } {
if (node.icon) {
const shape = resolveIcon(node.icon);
if (shape) {
return {
style: iconNodeStyle(preset, shape.style),
w: shape.w,
h: shape.h,
iconResolved: true,
};
}
}
// Generic shape by kind, sized to the label so a long label never overflows.
const w = Math.max(120, estimateLabelWidth(node.label) + 32);
return { style: genericNodeStyle(preset, node.kind), w, h: 60, iconResolved: false };
}
/** Rough rendered width of the longest label line at 12px (~0.6em/glyph). */
function estimateLabelWidth(label: string): number {
const lines = label.split(/\r?\n|&#xa;|<br\s*\/?>/i);
let longest = 0;
for (const l of lines) longest = Math.max(longest, l.trim().length);
return Math.ceil(longest * 12 * 0.6);
}
// --- graph -> ELK-JSON -----------------------------------------------------
interface ElkNode {
id: string;
width?: number;
height?: number;
x?: number;
y?: number;
children?: ElkNode[];
layoutOptions?: Record<string, string>;
}
interface ElkEdge {
id: string;
sources: string[];
targets: string[];
}
interface ElkGraph extends ElkNode {
edges?: ElkEdge[];
}
const ELK_DIRECTION: Record<string, string> = {
LR: "RIGHT",
RL: "LEFT",
TB: "DOWN",
BT: "UP",
};
/** Sizes resolved per node id (from resolveNodeStyle), fed to the ELK mapper. */
export interface NodeSize {
w: number;
h: number;
}
/**
* Build the ELK graph from the semantic graph + resolved node sizes. Compound
* group nodes nest their members (a group may itself nest in another group).
* `only` restricts the graph to a subset of node ids (used by the incremental
* path to lay out ONLY the new nodes). Layout HINTS (`layer`/`sameLayerAs`/
* `pinned`) are NOT encoded as ELK constraints here ELK's constraint knobs are
* unreliable across versions they are enforced deterministically AFTER layout
* by applyHints, which is exact and testable.
*/
export function graphToElk(
graph: Graph,
sizes: Map<string, NodeSize>,
opts: { only?: Set<string> } = {},
): ElkGraph {
const direction = ELK_DIRECTION[graph.direction ?? "LR"] ?? "RIGHT";
const only = opts.only;
const include = (id: string) => !only || only.has(id);
const makeNode = (n: GraphNode): ElkNode => {
const size = sizes.get(n.id) ?? { w: 140, h: 60 };
return { id: n.id, width: size.w, height: size.h };
};
// Group children nest under their group node; ungrouped nodes are roots.
const groupNode = new Map<string, ElkNode>();
const usedGroups = new Set<string>();
for (const g of graph.groups ?? []) {
const size = sizes.get(g.id) ?? { w: 200, h: 150 };
groupNode.set(g.id, {
id: g.id,
width: size.w,
height: size.h,
children: [],
layoutOptions: {
"elk.algorithm": "layered",
"elk.direction": direction,
"elk.padding": "[top=40,left=30,bottom=30,right=30]",
"elk.layered.spacing.nodeNodeBetweenLayers": "170",
"elk.spacing.nodeNode": "170",
},
});
}
const roots: ElkNode[] = [];
for (const n of graph.nodes) {
if (!include(n.id)) continue;
const en = makeNode(n);
if (n.group && groupNode.has(n.group)) {
groupNode.get(n.group)!.children!.push(en);
usedGroups.add(n.group);
} else {
roots.push(en);
}
}
// Nest group nodes into their parent group (a subnet inside a VPC); groups
// with no parent group become roots. Only groups that hold an included node.
const groupIdSet = new Set((graph.groups ?? []).map((g) => g.id));
for (const g of graph.groups ?? []) {
if (only && !usedGroups.has(g.id)) continue;
const en = groupNode.get(g.id)!;
if (g.group && groupIdSet.has(g.group) && g.group !== g.id && (!only || usedGroups.has(g.group))) {
groupNode.get(g.group)!.children!.push(en);
} else {
roots.push(en);
}
}
// Edges: endpoints may be nodes or groups; INCLUDE_CHILDREN spans the nesting.
const validIds = new Set<string>([
...graph.nodes.filter((n) => include(n.id)).map((n) => n.id),
...(graph.groups ?? []).map((g) => g.id),
]);
const edges: ElkEdge[] = [];
(graph.edges ?? []).forEach((e, i) => {
if (!validIds.has(e.from) || !validIds.has(e.to)) return;
edges.push({ id: `e${i}`, sources: [e.from], targets: [e.to] });
});
const rootOptions: Record<string, string> = {
"elk.algorithm": "layered",
"elk.direction": direction,
"elk.hierarchyHandling": "INCLUDE_CHILDREN",
"elk.layered.spacing.nodeNodeBetweenLayers": "170",
"elk.spacing.nodeNode": "170",
"elk.spacing.edgeNode": "40",
"elk.spacing.edgeEdge": "30",
"elk.padding": "[top=20,left=20,bottom=20,right=20]",
};
return { id: "root", layoutOptions: rootOptions, children: roots, edges };
}
/**
* Enforce the layout hints DETERMINISTICALLY on ELK's output (mutates `geo`):
* - `sameLayerAs`: snap the dependent node's LAYER-AXIS coordinate to its
* anchor's, so the pair lands in the same layer (x for LR/RL, y for TB/BT).
* `layer` groups nodes with the same index onto the same anchor coordinate.
* - `pinned`: override the node's coordinate with the exact pinned {x,y}.
* Applied only to top-level (ungrouped) nodes, whose ELK coords are absolute.
*/
export function applyHints(
graph: Graph,
geo: Map<string, { x: number; y: number; w: number; h: number }>,
): void {
const dir = graph.direction ?? "LR";
const layerAxis: "x" | "y" = dir === "TB" || dir === "BT" ? "y" : "x";
// The perpendicular (cross-layer) axis: members snapped onto one layer must be
// spread along THIS axis so they don't stack onto the same point.
const crossAxis: "x" | "y" = layerAxis === "x" ? "y" : "x";
const crossSize: "w" | "h" = crossAxis === "x" ? "w" : "h";
const grouped = new Set(
graph.nodes.filter((n) => n.group).map((n) => n.id),
);
// sameLayerAs / layer: co-assign the layer-axis coordinate.
// Build the effective layer key per node, then pick a representative coord.
const layerKeyOf = new Map<string, string>();
const explicitLayer = new Map<string, number>();
for (const n of graph.nodes) if (n.layer != null) explicitLayer.set(n.id, n.layer);
const byId = new Map(graph.nodes.map((n) => [n.id, n]));
const resolveKey = (n: GraphNode): string | null => {
if (explicitLayer.has(n.id)) return `L${explicitLayer.get(n.id)}`;
const seen = new Set<string>([n.id]);
let cur: GraphNode | undefined = n;
while (cur && cur.sameLayerAs != null && !seen.has(cur.sameLayerAs)) {
seen.add(cur.sameLayerAs);
const t = byId.get(cur.sameLayerAs);
if (!t) break;
if (explicitLayer.has(t.id)) return `L${explicitLayer.get(t.id)}`;
cur = t;
}
// A sameLayerAs chain with no explicit layer: key on the chain's root id.
if (n.sameLayerAs != null) {
let root = n.id;
const s2 = new Set<string>([n.id]);
let c: GraphNode | undefined = n;
while (c && c.sameLayerAs != null && !s2.has(c.sameLayerAs)) {
s2.add(c.sameLayerAs);
root = c.sameLayerAs;
c = byId.get(c.sameLayerAs);
}
return `C${root}`;
}
return null;
};
for (const n of graph.nodes) {
if (grouped.has(n.id)) continue; // group children are relative — skip
const key = resolveKey(n);
if (key) layerKeyOf.set(n.id, key);
}
// Group members of each layer key so we can snap AND spread them together.
const membersOf = new Map<string, string[]>();
for (const n of graph.nodes) {
const key = layerKeyOf.get(n.id);
if (key == null) continue;
if (!geo.has(n.id)) continue;
(membersOf.get(key) ?? membersOf.set(key, []).get(key)!).push(n.id);
}
// For each layer key: snap every member to the FIRST member's layer-axis coord,
// then SPREAD them along the perpendicular (cross-layer) axis with a >=
// MIN_SHAPE_GAP gap. Without the spread, a sameLayerAs chain whose nodes ELK
// happened to give the same cross-axis coordinate would collapse onto one point
// -> shape-overlap + edge-through-shape quality warnings (breaking the
// "0 warnings by construction" guarantee for these AUTO-positioned hints). We
// start from the members' minimum cross-axis coord and stack them with a gap
// of MIN_SHAPE_GAP beyond each shape's cross-axis size.
for (const [key, members] of membersOf) {
if (members.length === 0) continue;
// Snap layer-axis coord to the first member.
const repCoord = geo.get(members[0])![layerAxis];
// Preserve the members' existing relative order along the cross axis so the
// spread stays visually stable, then re-lay them contiguously.
const sorted = [...members].sort(
(a, b) => geo.get(a)![crossAxis] - geo.get(b)![crossAxis],
);
let cursor = geo.get(sorted[0])![crossAxis];
for (const id of sorted) {
const g = geo.get(id)!;
g[layerAxis] = repCoord;
g[crossAxis] = cursor;
cursor += g[crossSize] + MIN_SHAPE_GAP;
}
void key;
}
// pinned: exact override (wins over any layer snap). Explicit user coordinates
// are user intent, but CLAMP to non-negative so an out-of-bounds pin (e.g.
// x:-500) never renders off-canvas. Two user-pinned nodes at the same point is
// user error the server can't silently relocate — the assembler docstring
// documents that explicit pins are user-directed and MAY warn (see #423/#425
// acceptance: the "0 quality-warnings by construction" guarantee is for
// AUTO-LAYOUT, not for coordinates the user pinned by hand).
for (const n of graph.nodes) {
if (!n.pinned) continue;
const px = Math.max(0, n.pinned.x);
const py = Math.max(0, n.pinned.y);
const g = geo.get(n.id);
if (g) {
g.x = px;
g.y = py;
} else {
const sz = { w: 140, h: 60 };
geo.set(n.id, { x: px, y: py, w: sz.w, h: sz.h });
}
}
}
/**
* Incremental variant of applyHints: apply `pinned` only to NEW nodes (those
* absent from `existing`); an existing node's coordinates are NEVER changed
* (acceptance #3). sameLayerAs/layer snapping is intentionally skipped in the
* incremental path moving a new node's layer axis could still be desired, but
* it must never move an existing cell, so we keep the incremental contract
* simple: existing cells are frozen, new pinned nodes honour their pin.
*/
export function applyHintsForNew(
graph: Graph,
geo: Map<string, { x: number; y: number; w: number; h: number }>,
existing: Map<string, { x: number; y: number }>,
): void {
for (const n of graph.nodes) {
if (existing.has(n.id)) continue; // never move an existing cell
if (!n.pinned) continue;
const px = Math.max(0, n.pinned.x); // clamp out-of-bounds pins non-negative
const py = Math.max(0, n.pinned.y);
const g = geo.get(n.id);
if (g) {
g.x = px;
g.y = py;
} else {
geo.set(n.id, { x: px, y: py, w: 140, h: 60 });
}
}
}
// --- layout runner ---------------------------------------------------------
const ELK_TIMEOUT_MS = 5000;
/**
* Run ELK over the mapped graph and return computed geometry per id (coords are
* parent-relative, matching mxGraph's convention for container children). On any
* ELK failure/timeout the returned map is empty and the caller falls back to a
* deterministic grid placement (so the write never fails on a layout hiccup).
*/
export async function runElk(
elk: ElkGraph,
): Promise<Map<string, { x: number; y: number; w: number; h: number }>> {
const geo = new Map<string, { x: number; y: number; w: number; h: number }>();
let timer: ReturnType<typeof setTimeout> | undefined;
try {
const Ctor: any = (ELK as any).default ?? ELK;
const inst = new Ctor();
const timeout = new Promise<never>((_, reject) => {
timer = setTimeout(() => reject(new Error("ELK timed out")), ELK_TIMEOUT_MS);
});
const laid = (await Promise.race([inst.layout(elk as any), timeout])) as ElkGraph;
const walk = (n: ElkNode) => {
if (n.id !== "root") {
geo.set(n.id, {
x: Math.round(n.x ?? 0),
y: Math.round(n.y ?? 0),
w: Math.round(n.width ?? 140),
h: Math.round(n.height ?? 60),
});
}
for (const c of n.children ?? []) walk(c);
};
walk(laid);
} catch {
return new Map(); // best-effort: empty -> caller uses fallback grid.
} finally {
if (timer) clearTimeout(timer);
}
return geo;
}
// --- XML assembler ---------------------------------------------------------
/** Order groups so a parent group always precedes its nested children. */
function topoSortGroups(groups: GraphGroup[], groupIds: Set<string>): GraphGroup[] {
const byId = new Map(groups.map((g) => [g.id, g]));
const out: GraphGroup[] = [];
const done = new Set<string>();
const visit = (g: GraphGroup, stack: Set<string>) => {
if (done.has(g.id)) return;
if (stack.has(g.id)) return; // cycle guard
stack.add(g.id);
if (g.group && groupIds.has(g.group) && g.group !== g.id) {
const parent = byId.get(g.group);
if (parent) visit(parent, stack);
}
stack.delete(g.id);
if (!done.has(g.id)) {
done.add(g.id);
out.push(g);
}
};
for (const g of groups) visit(g, new Set());
return out;
}
function xmlEscapeAttr(s: string): string {
return s
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;")
.replace(/\r\n|\r|\n/g, "&#xa;"); // literal newline -> the linter-approved entity
}
export interface AssembleResult {
modelXml: string;
iconsResolved: number;
iconsMissing: string[];
}
/**
* Assemble the final mxGraphModel XML from the graph + resolved styles/coords.
* Guarantees BY CONSTRUCTION that the #423 linter passes:
* - id=0 and id=1(parent=0) sentinels;
* - each node/group is vertex="1" (containers get container=1 via the style);
* - each edge is edge="1" with a child <mxGeometry relative="1" as="geometry"/>;
* - group children set parent=<groupId> and RELATIVE coords; an edge between
* two different parents is parent="1";
* - labels are XML-escaped and any newline is &#xa;.
* `geo` may be empty (ELK failed) then a deterministic grid is used so the
* output is still valid and non-overlapping (>=170px stride).
*
* QUALITY-WARNING GUARANTEE: the "0 quality-warnings by construction" promise
* holds for AUTO-LAYOUT ELK spacing plus applyHints' cross-axis spread for the
* server-positioned `layer`/`sameLayerAs` hints keep shapes >=MIN_SHAPE_GAP
* apart. It does NOT extend to explicit `pinned` coordinates: those are
* user-directed, so two nodes the user pins to the same/overlapping point are
* user error the server honours verbatim (only clamped non-negative) and MAY
* therefore produce a quality warning.
*/
export function assembleModel(
graph: Graph,
opts: {
preset: PresetData;
styles: Map<string, { style: string; w: number; h: number; iconResolved: boolean }>;
geo: Map<string, { x: number; y: number; w: number; h: number }>;
},
): AssembleResult {
const { preset, styles, geo } = opts;
const groupIds = new Set((graph.groups ?? []).map((g) => g.id));
const nodeById = new Map(graph.nodes.map((n) => [n.id, n]));
// Fallback grid when ELK produced nothing: lay ungrouped nodes on a grid with
// a 190px stride (>150 gap). Grouped nodes/groups are placed inside their group.
const fallback = geo.size === 0;
const gridPos = (i: number) => ({ x: 40 + (i % 5) * 200, y: 40 + Math.floor(i / 5) * 140 });
const cells: string[] = ['<mxCell id="0"/>', '<mxCell id="1" parent="0"/>'];
// Groups first (they are parents of their members). A nested group sets
// parent=<parentGroupId>; emit parents before children so parent-exists holds.
let gi = 0;
const groupGeo = new Map<string, { x: number; y: number; w: number; h: number }>();
const orderedGroups = topoSortGroups(graph.groups ?? [], groupIds);
for (const g of orderedGroups) {
const gg = geo.get(g.id) ?? { ...gridPos(gi++), w: 320, h: 220 };
groupGeo.set(g.id, gg);
const style = groupStyle(preset);
const gParent = g.group && groupIds.has(g.group) && g.group !== g.id ? g.group : "1";
cells.push(
`<mxCell id="${xmlEscapeAttr(g.id)}" value="${xmlEscapeAttr(g.label)}" style="${style}" vertex="1" parent="${xmlEscapeAttr(gParent)}">` +
`<mxGeometry x="${gg.x}" y="${gg.y}" width="${gg.w}" height="${gg.h}" as="geometry"/></mxCell>`,
);
}
// Nodes. A grouped node's coords are RELATIVE to its group (ELK already
// returns child coords relative to the parent; for the fallback grid we place
// children on a small in-group grid).
let ungrouped = (graph.groups?.length ?? 0);
const inGroupIndex = new Map<string, number>();
let iconsResolved = 0;
const iconsMissing: string[] = [];
for (const n of graph.nodes) {
const st = styles.get(n.id)!;
if (n.icon) {
if (st.iconResolved) iconsResolved++;
else iconsMissing.push(n.id);
}
let x: number;
let y: number;
const g = geo.get(n.id);
if (g && !fallback) {
x = g.x;
y = g.y;
} else if (n.group && groupIds.has(n.group)) {
const k = inGroupIndex.get(n.group) ?? 0;
inGroupIndex.set(n.group, k + 1);
x = 30 + (k % 3) * 180;
y = 40 + Math.floor(k / 3) * 120;
} else {
const p = gridPos(ungrouped++);
x = p.x;
y = p.y;
}
const parent = n.group && groupIds.has(n.group) ? n.group : "1";
cells.push(
`<mxCell id="${xmlEscapeAttr(n.id)}" value="${xmlEscapeAttr(n.label)}" style="${st.style}" vertex="1" parent="${xmlEscapeAttr(parent)}">` +
`<mxGeometry x="${x}" y="${y}" width="${st.w}" height="${st.h}" as="geometry"/></mxCell>`,
);
}
// Edges. parent="1" whenever the two endpoints have different container
// parents (or either is a group); otherwise the shared group id.
(graph.edges ?? []).forEach((e, i) => {
const style = edgeStyle(preset, e.kind);
const fromNode = nodeById.get(e.from);
const toNode = nodeById.get(e.to);
const fromParent = fromNode?.group && groupIds.has(fromNode.group) ? fromNode.group : "1";
const toParent = toNode?.group && groupIds.has(toNode.group) ? toNode.group : "1";
const parent = fromParent === toParent ? fromParent : "1";
const label = e.label ? ` value="${xmlEscapeAttr(e.label)}"` : "";
cells.push(
`<mxCell id="ge${i}"${label} style="${style}" edge="1" parent="${xmlEscapeAttr(parent)}" ` +
`source="${xmlEscapeAttr(e.from)}" target="${xmlEscapeAttr(e.to)}">` +
`<mxGeometry relative="1" as="geometry"/></mxCell>`,
);
});
const modelAttrs =
'dx="0" dy="0" grid="1" gridSize="10" page="1" pageWidth="850" pageHeight="1100" adaptiveColors="auto"';
const modelXml = `<mxGraphModel ${modelAttrs}><root>${cells.join("")}</root></mxGraphModel>`;
return { modelXml, iconsResolved, iconsMissing };
}
// --- incremental merge -----------------------------------------------------
let _mergeWindow: any = null;
function mergeWindow(): any {
if (!_mergeWindow) _mergeWindow = new JSDOM("").window;
return _mergeWindow;
}
/**
* Merge the freshly-assembled graph XML with the EXISTING diagram model so an
* incremental "add a node" call never drops a hand-placed cell. `assembleModel`
* emits ONLY the passed graph's cells; on its own it would replace the whole
* model, wiping any existing cell the caller didn't re-list. This splices every
* existing cell that the graph does NOT re-list (preserved verbatim: coords,
* style, edges) into the assembled root:
* - id in the graph -> the graph's (re-laid) cell wins (already assembled;
* coords are frozen for existing ids via the incremental geo path);
* - id NOT in the graph -> the existing cell is preserved verbatim;
* - a graph node absent from the existing model -> added (offset clear).
* The sentinels ("0"/"1") come from the assembled model and are never doubled.
*/
function mergeExistingCells(
assembledXml: string,
existingModelXml: string,
graph: Graph,
): string {
const win = mergeWindow();
const parser = new win.DOMParser();
const existingDoc = parser.parseFromString(existingModelXml, "application/xml");
if (existingDoc.getElementsByTagName("parsererror").length > 0) {
// Existing model unreadable: fall back to the assembled model alone (still a
// valid diagram — better than throwing on a corrupt prior file).
return assembledXml;
}
const assembledDoc = parser.parseFromString(assembledXml, "application/xml");
const root = assembledDoc.getElementsByTagName("root")[0];
if (!root) return assembledXml;
// Ids the assembled model already emitted (graph nodes/groups/edges + sentinels).
const assembledIds = new Set<string>();
for (const el of Array.from(root.getElementsByTagName("mxCell")) as any[]) {
const id = el.getAttribute("id");
if (id) assembledIds.add(id);
}
// The graph's own ids: any existing cell with one of these is superseded by the
// assembled version and must NOT be re-imported.
const graphIds = new Set<string>([
...graph.nodes.map((n) => n.id),
...(graph.groups ?? []).map((g) => g.id),
]);
const existingCells = Array.from(
existingDoc.getElementsByTagName("mxCell"),
) as any[];
for (const el of existingCells) {
const id = el.getAttribute("id") ?? "";
if (id === "0" || id === "1") continue; // sentinels come from the assembled model
if (graphIds.has(id)) continue; // graph re-lists it -> assembled version wins
if (assembledIds.has(id)) continue; // id collision guard -> keep assembled
root.appendChild(assembledDoc.importNode(el, true));
assembledIds.add(id);
}
const ser = new win.XMLSerializer();
return ser.serializeToString(assembledDoc.documentElement);
}
// --- top-level: graph -> mxGraphModel XML ----------------------------------
export interface BuildFromGraphResult {
modelXml: string;
iconsResolved: number;
iconsMissing: string[];
layout: LayoutMode;
}
/**
* The full server-side pipeline: validate -> resolve styles/icons -> map to ELK
* -> run ELK (or fall back) -> assemble linter-clean XML. `existingCoords` is
* supplied for `layout:"incremental"` (the coordinates of the diagram's current
* cells, so they are preserved and only new nodes are placed). `existingModelXml`
* is the current diagram's full model XML in incremental mode every existing
* cell the graph does NOT re-list is MERGED back in verbatim so a hand-placed
* cell is never dropped (WARNING #4). Pure no network.
*/
export async function buildFromGraph(
graph: Graph,
layout: LayoutMode = "full",
existingCoords?: Map<string, { x: number; y: number }>,
existingModelXml?: string,
): Promise<BuildFromGraphResult> {
validateGraph(graph);
const preset = getPreset(graph.preset);
// Resolve every node's style + size (icon or generic-by-kind).
const styles = new Map<
string,
{ style: string; w: number; h: number; iconResolved: boolean }
>();
const sizes = new Map<string, NodeSize>();
for (const n of graph.nodes) {
const s = resolveNodeStyle(preset, n);
styles.set(n.id, s);
sizes.set(n.id, { w: s.w, h: s.h });
}
// Group sizes: seed a min box; ELK computes the real size when it lays out.
for (const g of graph.groups ?? []) sizes.set(g.id, { w: 240, h: 180 });
let geo = new Map<string, { x: number; y: number; w: number; h: number }>();
if (layout === "incremental" && existingCoords && existingCoords.size > 0) {
// INCREMENTAL: keep every existing cell's coords VERBATIM (acceptance #3 —
// never move a hand-arranged cell) and lay out ONLY the new nodes, then
// offset that block clear of the existing bbox so nothing overlaps.
for (const [id, c] of existingCoords) {
const sz = sizes.get(id) ?? { w: 140, h: 60 };
geo.set(id, { x: c.x, y: c.y, w: sz.w, h: sz.h });
}
const newIds = new Set(
graph.nodes.filter((n) => !existingCoords.has(n.id)).map((n) => n.id),
);
if (newIds.size > 0) {
const elk = graphToElk(graph, sizes, { only: newIds });
const laid = await runElk(elk);
// Place the new block below the existing content (a clear >=170px gap).
let maxY = 0;
for (const c of existingCoords.values()) maxY = Math.max(maxY, c.y);
const offsetY = maxY + 200;
for (const [id, g] of laid) {
if (newIds.has(id)) geo.set(id, { ...g, y: g.y + offsetY });
else if (!geo.has(id)) geo.set(id, g); // a new group container
}
}
// Hints still apply to NEW pinned nodes only (existing ones stay put).
applyHintsForNew(graph, geo, existingCoords);
} else if (layout !== "none") {
const elk = graphToElk(graph, sizes);
geo = await runElk(elk);
applyHints(graph, geo);
} else if (existingCoords) {
// layout:"none" with prior coords -> keep them verbatim.
for (const [id, c] of existingCoords) {
const sz = sizes.get(id) ?? { w: 140, h: 60 };
geo.set(id, { x: c.x, y: c.y, w: sz.w, h: sz.h });
}
}
const assembled = assembleModel(graph, { preset, styles, geo });
// In incremental mode, splice back every existing cell the graph didn't
// re-list so an "add one node" call preserves the user's manual layout.
let modelXml = assembled.modelXml;
if (
layout === "incremental" &&
existingModelXml &&
existingCoords &&
existingCoords.size > 0
) {
modelXml = mergeExistingCells(modelXml, existingModelXml, graph);
}
return {
modelXml,
iconsResolved: assembled.iconsResolved,
iconsMissing: assembled.iconsMissing,
layout,
};
}
-347
View File
@@ -1,347 +0,0 @@
// Pure Mermaid `flowchart` -> graph-JSON parser for `drawioFromMermaid` (issue
// #425, stage 3, OPTIONAL). The escape clause in the issue: convert WITHOUT
// Electron/draw.io-CLI, so a pure text parser only. It handles the common wiki
// flowchart subset — node shapes, labelled/dashed edges, subgraphs (-> groups),
// and the direction header — and emits a Graph the drawioFromGraph pipeline
// renders as an EDITABLE draw.io diagram. Anything beyond flowchart (sequence /
// class / state) throws a clear error so the model falls back to drawioFromGraph.
//
// DELIBERATELY NARROW: this is not a full Mermaid grammar (Mermaid's own parser
// is a 100KB+ browser dependency). It covers `flowchart`/`graph` with the node
// shapes and edge arrows that show up in practice; unusual syntax is skipped
// rather than mis-parsed, and a diagram that yields no nodes throws.
import type { Graph, GraphNode, GraphEdge, GraphGroup } from "./drawio-graph.js";
export class MermaidParseError extends Error {
constructor(message: string) {
super(`drawioFromMermaid: ${message}`);
this.name = "MermaidParseError";
}
}
// Input-size bounds applied BEFORE parsing. Without them a pathological mermaid
// string (e.g. 300000 connection lines, or 20000 nested `subgraph`s) builds a
// huge intermediate node/edge/group structure that OOM-crashes the worker — the
// downstream validateGraph caps in drawio-graph can't help because the parser
// exhausts the heap constructing the intermediate FIRST. These caps reject the
// over-limit input fast, before a single line is parsed.
const MAX_MERMAID_CHARS = 200_000; // ~200 KB of source is far beyond any real diagram.
const MAX_MERMAID_LINES = 20_000;
const MAX_MERMAID_GROUPS = 500; // parity with drawio-graph's MAX_GRAPH_GROUPS.
// Per connection line, the number of chained nodes we will expand (`A-->B-->C`).
const MAX_CHAIN_NODES = 500;
const DIRECTIONS: Record<string, Graph["direction"]> = {
LR: "LR",
RL: "RL",
TB: "TB",
TD: "TB",
BT: "BT",
};
/**
* Node-shape delimiters -> a semantic `kind`. Mermaid encodes shape in the
* bracket style; we map the common ones to the palette kinds so the diagram is
* colored meaningfully (a decision/diamond -> queue, a database cylinder -> db,
* a rounded/stadium -> service, a subroutine/hexagon -> gateway, default rect ->
* service). The label text lives between the delimiters.
*/
interface ShapeDef {
open: string;
close: string;
kind: string;
}
// Order matters: longer/multi-char delimiters first so "([" beats "(".
const SHAPES: ShapeDef[] = [
{ open: "([", close: "])", kind: "service" }, // stadium
{ open: "[[", close: "]]", kind: "gateway" }, // subroutine
{ open: "[(", close: ")]", kind: "db" }, // cylinder-ish / database
{ open: "((", close: "))", kind: "external" }, // circle
{ open: "{{", close: "}}", kind: "gateway" }, // hexagon
{ open: "[", close: "]", kind: "service" }, // rectangle
{ open: "(", close: ")", kind: "service" }, // rounded
{ open: "{", close: "}", kind: "queue" }, // rhombus / decision
{ open: ">", close: "]", kind: "external" }, // asymmetric flag
];
/** Strip Mermaid label quoting/escapes and normalise whitespace. */
function cleanLabel(raw: string): string {
let s = raw.trim();
if (
(s.startsWith('"') && s.endsWith('"')) ||
(s.startsWith("'") && s.endsWith("'"))
) {
s = s.slice(1, -1);
}
return s.replace(/<br\s*\/?>/gi, " ").replace(/\s+/g, " ").trim();
}
/** A single edge-arrow spec: its regex and the resulting edge `kind`. */
interface ArrowDef {
re: RegExp;
kind: string;
}
// Dotted arrows (`-.->`) -> async; thick (`==>`) stay sync; normal `-->`/`---`.
// Each captures an optional `|label|` OR inline label between the two arrow
// halves. Applied to the segment between two node tokens.
const ARROWS: ArrowDef[] = [
{ re: /-\.->|-\.-/, kind: "async" },
{ re: /==>|===/, kind: "sync" },
{ re: /-->|---/, kind: "sync" },
];
interface ParsedRef {
id: string;
node?: GraphNode;
}
/**
* Parse a single node token like `A`, `A[Label]`, `db[(Orders)]`, `d{Choose}`.
* Returns the id and, when the token declares a shape/label, a GraphNode.
*/
function parseNodeToken(token: string): ParsedRef | null {
const t = token.trim();
if (t === "") return null;
for (const shape of SHAPES) {
const oi = t.indexOf(shape.open);
if (oi <= 0) continue;
if (!t.endsWith(shape.close)) continue;
const id = t.slice(0, oi).trim();
const label = cleanLabel(t.slice(oi + shape.open.length, t.length - shape.close.length));
if (!id) return null;
return { id, node: { id, label: label || id, kind: shape.kind } };
}
// Bare id (no shape declared here — may be defined elsewhere).
if (/^[A-Za-z0-9_.-]+$/.test(t)) return { id: t };
return null;
}
/**
* Split a connection line into [leftToken, arrowSegment, rightToken]. Returns
* null if the line has no arrow. The arrow segment may embed a label as
* `-->|text|` or `-- text -->`.
*/
function splitConnection(
line: string,
): { left: string; right: string; kind: string; label?: string } | null {
for (const arrow of ARROWS) {
// Find the arrow occurrence. Support a mid-arrow label: `A -- text --> B`.
const m = arrow.re.exec(line);
if (!m) continue;
const idx = m.index;
let left = line.slice(0, idx).trim();
let rest = line.slice(idx + m[0].length).trim();
let label: string | undefined;
// Pipe label: `-->|HTTPS| B`.
const pipe = /^\|([^|]*)\|\s*(.*)$/.exec(rest);
if (pipe) {
label = cleanLabel(pipe[1]);
rest = pipe[2].trim();
}
// Mid-arrow label on the left side: `A -- text` before the arrow half.
const midLeft = /^(.*?)\s*--\s*(.+)$/.exec(left);
if (!label && midLeft && /-\.|--|==/.test(line.slice(0, idx))) {
// Only treat as a label when there's clearly text after `--`.
if (!/[\[\](){}]/.test(midLeft[2])) {
left = midLeft[1].trim();
label = cleanLabel(midLeft[2]);
}
}
if (!left || !rest) return null;
return { left, right: rest, kind: arrow.kind, label };
}
return null;
}
/**
* Parse Mermaid flowchart text into a Graph. Handles the header
* (`flowchart LR` / `graph TD`), `subgraph <id>[title] … end` blocks (-> groups),
* node declarations, and connection lines. Throws MermaidParseError for a
* non-flowchart diagram or when nothing parses.
*/
export function mermaidToGraph(mermaid: string): Graph {
if (typeof mermaid !== "string" || mermaid.trim() === "") {
throw new MermaidParseError("empty mermaid input");
}
// Size guards FIRST — bound the raw input before building any intermediate.
if (mermaid.length > MAX_MERMAID_CHARS) {
throw new MermaidParseError(
`input is ${mermaid.length} chars (max ${MAX_MERMAID_CHARS}); split the diagram or use drawioFromGraph`,
);
}
const rawLines = mermaid.split(/\r?\n/);
if (rawLines.length > MAX_MERMAID_LINES) {
throw new MermaidParseError(
`input has ${rawLines.length} lines (max ${MAX_MERMAID_LINES}); split the diagram or use drawioFromGraph`,
);
}
const nodes = new Map<string, GraphNode>();
const groups: GraphGroup[] = [];
const edges: GraphEdge[] = [];
let direction: Graph["direction"] = "LR";
let sawHeader = false;
// Stack of active subgraph ids (nesting); the top is the current group.
const groupStack: string[] = [];
let anonGroup = 0;
const ensureNode = (ref: ParsedRef) => {
const existing = nodes.get(ref.id);
if (ref.node) {
if (existing) {
// Fill in a label/kind if this token declared a shape and the prior didn't.
if (existing.label === existing.id && ref.node.label !== ref.node.id)
existing.label = ref.node.label;
if (!existing.kind) existing.kind = ref.node.kind;
} else {
nodes.set(ref.id, { ...ref.node });
}
} else if (!existing) {
nodes.set(ref.id, { id: ref.id, label: ref.id, kind: "service" });
}
// Assign to the current subgraph if inside one and not yet grouped.
const cur = groupStack[groupStack.length - 1];
const n = nodes.get(ref.id)!;
if (cur && n.group == null) n.group = cur;
};
for (const raw of rawLines) {
let line = raw.trim();
if (line === "" || line.startsWith("%%")) continue; // blank / comment
// Header.
const header = /^(flowchart|graph)\s+([A-Za-z]{2})\b/.exec(line);
if (header) {
sawHeader = true;
const dir = DIRECTIONS[header[2].toUpperCase()];
if (dir) direction = dir;
continue;
}
if (/^(sequenceDiagram|classDiagram|stateDiagram|erDiagram|gantt|pie|journey)\b/.test(line)) {
throw new MermaidParseError(
`only 'flowchart'/'graph' is supported (got '${line.split(/\s+/)[0]}'); use drawioFromGraph instead`,
);
}
// Subgraph open: `subgraph id [Title]` or `subgraph Title`.
const sg = /^subgraph\s+(.+)$/.exec(line);
if (sg) {
const spec = sg[1].trim();
let id: string;
let label: string;
const bracket = /^([A-Za-z0-9_.-]+)\s*\[(.+)\]$/.exec(spec);
if (bracket) {
id = bracket[1];
label = cleanLabel(bracket[2]);
} else if (/^[A-Za-z0-9_.-]+$/.test(spec)) {
id = spec;
label = spec;
} else {
id = `sg${anonGroup++}`;
label = cleanLabel(spec);
}
if (!groups.some((g) => g.id === id)) {
if (groups.length >= MAX_MERMAID_GROUPS) {
throw new MermaidParseError(
`too many subgraphs (max ${MAX_MERMAID_GROUPS}); use drawioFromGraph for a diagram this large`,
);
}
groups.push({ id, label, kind: "group" });
}
groupStack.push(id);
continue;
}
if (/^end\b/.test(line)) {
groupStack.pop();
continue;
}
// `direction LR` inside a subgraph — apply to the top-level direction.
const innerDir = /^direction\s+([A-Za-z]{2})\b/.exec(line);
if (innerDir) {
const dir = DIRECTIONS[innerDir[1].toUpperCase()];
if (dir) direction = dir;
continue;
}
// Style/class/click directives: ignore (no visual mapping in our palette).
if (/^(style|classDef|class|click|linkStyle)\b/.test(line)) continue;
// Strip a trailing semicolon.
if (line.endsWith(";")) line = line.slice(0, -1).trim();
// Connection line (possibly chained: A --> B --> C).
const conn = splitConnection(line);
if (conn) {
// Handle a simple chain by re-splitting the right side.
let leftTok = conn.left;
let seg: typeof conn | null = conn;
let guard = 0;
while (seg) {
if (guard++ >= MAX_CHAIN_NODES) {
// Don't silently drop the tail of an over-long chain — surface it so
// the model knows the diagram was too large rather than getting a
// quietly-truncated result.
throw new MermaidParseError(
`a single connection chain exceeds ${MAX_CHAIN_NODES} nodes; split it or use drawioFromGraph`,
);
}
const leftRef = parseNodeToken(leftTok);
// The right side may itself contain another arrow (a chain).
const nextSeg = splitConnection(seg.right);
const rightTokenStr = nextSeg ? seg.right.slice(0, splitIndex(seg.right)) : seg.right;
const rightRef = parseNodeToken(nextSeg ? nextSeg.left : seg.right);
if (leftRef && rightRef) {
ensureNode(leftRef);
ensureNode(rightRef);
edges.push({
from: leftRef.id,
to: rightRef.id,
label: seg.label,
kind: seg.kind,
});
}
if (!nextSeg) break;
leftTok = nextSeg.left;
seg = nextSeg;
void rightTokenStr;
}
continue;
}
// Standalone node declaration `A[Label]` OR a bare member ref `C` inside a
// subgraph (which claims that node for the current group).
const nodeRef = parseNodeToken(line);
if (nodeRef && (nodeRef.node || groupStack.length > 0)) {
ensureNode(nodeRef);
continue;
}
// Unknown line: skip silently (robustness over strictness).
}
if (!sawHeader && nodes.size === 0) {
throw new MermaidParseError(
"input does not look like a mermaid flowchart (no 'flowchart'/'graph' header and no nodes)",
);
}
if (nodes.size === 0) {
throw new MermaidParseError("no nodes parsed from the flowchart");
}
const graph: Graph = {
nodes: Array.from(nodes.values()),
direction,
};
if (groups.length > 0) graph.groups = groups;
if (edges.length > 0) graph.edges = edges;
return graph;
}
/** Index of the first arrow in a segment (for chain splitting). */
function splitIndex(s: string): number {
let best = -1;
for (const arrow of ARROWS) {
const m = arrow.re.exec(s);
if (m && (best === -1 || m.index < best)) best = m.index;
}
return best === -1 ? s.length : best;
}
-151
View File
@@ -1,151 +0,0 @@
// Semantic color/line presets for the graph tools (issue #425, stage 3). The
// PALETTE is DATA (packages/mcp/data/drawio-presets.json), not code: a node
// `kind` maps to a { fillColor, strokeColor, fontColor } slot and an edge `kind`
// maps to line-style props, per named preset (`default` / `dark` /
// `colorblind-safe`). This module only loads that data and turns a slot into a
// draw.io style fragment. The INVARIANT of the graph tools is that the model
// never sees a style string — it names a `kind`, the server picks the slot.
//
// Loading mirrors drawio-shapes.ts: the JSON is read once via `import.meta.url`
// relative to the built module. That is why this module (and drawio-graph.ts
// which imports it) is reached ONLY through client.ts's ESM build and never
// value-imported into the zod-agnostic tool-specs.ts (which the in-app server
// type-checks under module:commonjs, where `import.meta` is a TS1343 error).
import { readFileSync } from "node:fs";
/** A node color slot: the three draw.io color values for a `kind`. */
export interface NodeSlot {
fillColor: string;
strokeColor: string;
fontColor: string;
}
/** An edge line style: the extra style props appended for an edge `kind`. */
export interface EdgeStyle {
props: string;
}
export interface PresetData {
canvasDark: boolean;
okabeIto?: string[];
nodes: Record<string, NodeSlot>;
edges: Record<string, EdgeStyle>;
edgeDefault: { strokeColor: string; fontColor: string };
group: { strokeColor: string; fontColor: string };
}
interface PresetsFile {
presets: Record<string, PresetData>;
}
/** The three shipped preset names. */
export const PRESET_NAMES = ["default", "dark", "colorblind-safe"] as const;
export type PresetName = (typeof PRESET_NAMES)[number];
/** Every node `kind` the base palette defines (also the generic-shape kinds). */
export const NODE_KINDS = [
"service",
"db",
"queue",
"gateway",
"error",
"external",
"security",
] as const;
export type NodeKind = (typeof NODE_KINDS)[number];
/** Edge `kind`s the palette styles; anything else falls back to `sync`. */
export const EDGE_KINDS = ["sync", "async", "error"] as const;
export type EdgeKind = (typeof EDGE_KINDS)[number];
let _presets: Record<string, PresetData> | null = null;
function presetsPath(): URL {
// build/lib/drawio-presets.js -> ../../data/drawio-presets.json
return new URL("../../data/drawio-presets.json", import.meta.url);
}
/** Load + parse the bundled preset table once, then cache it. */
export function loadPresets(): Record<string, PresetData> {
if (_presets) return _presets;
const json = readFileSync(presetsPath(), "utf-8");
const parsed = JSON.parse(json) as PresetsFile;
_presets = parsed.presets;
return _presets;
}
/** Resolve a preset by name, defaulting to `default` for an unknown name. */
export function getPreset(name?: string): PresetData {
const presets = loadPresets();
if (name && presets[name]) return presets[name];
return presets["default"];
}
/** The slot for a node `kind` in a preset, falling back to `service`. */
export function nodeSlot(preset: PresetData, kind?: string): NodeSlot {
if (kind && preset.nodes[kind]) return preset.nodes[kind];
return preset.nodes["service"];
}
/**
* Build the draw.io style string for a GENERIC (no-icon) node of a given kind.
* A rounded rectangle carrying the slot's fill/stroke/font. `whiteSpace=wrap`
* and `html=1` let a long label wrap inside the shape (the assembler also sizes
* the shape to the label, so the linter's label-overflow warning never fires).
*/
export function genericNodeStyle(preset: PresetData, kind?: string): string {
const s = nodeSlot(preset, kind);
return (
`rounded=1;whiteSpace=wrap;html=1;` +
`fillColor=${s.fillColor};strokeColor=${s.strokeColor};fontColor=${s.fontColor};`
);
}
/**
* Overlay the preset's node slot colors onto a resolved ICON style-string
* (from the shape catalog). An AWS/Azure icon carries its OWN mandatory
* fill/stroke (the category color / white outline) that MUST NOT be recolored,
* so for an icon we only ensure a readable fontColor when the preset is dark;
* otherwise the icon style is returned verbatim. Keeping the icon's own colors
* is deliberate: recoloring an AWS service icon breaks its category semantics.
*/
export function iconNodeStyle(preset: PresetData, iconStyle: string): string {
if (!preset.canvasDark) return iconStyle;
// On a dark canvas an icon's fontColor is usually a dark ink that vanishes;
// append a light fontColor (icons put their label BELOW the glyph, so this
// only affects the caption, never the glyph fill).
if (/fontColor=/.test(iconStyle)) {
return iconStyle.replace(/fontColor=[^;]*/, "fontColor=#e0e0e0");
}
return iconStyle + (iconStyle.endsWith(";") ? "" : ";") + "fontColor=#e0e0e0;";
}
/**
* Build the draw.io style for an edge of a given `kind`. Base is an orthogonal
* connector (edgeStyle=orthogonalEdgeStyle) with rounded corners and an open
* arrowhead, plus the preset's default stroke/font, then the kind's extra props
* (dashed / colored) overlaid. An unknown kind falls back to `sync` (solid).
*/
export function edgeStyle(preset: PresetData, kind?: string): string {
const k = kind && preset.edges[kind] ? kind : "sync";
const base =
`edgeStyle=orthogonalEdgeStyle;rounded=1;html=1;endArrow=open;` +
`strokeColor=${preset.edgeDefault.strokeColor};fontColor=${preset.edgeDefault.fontColor};`;
return base + preset.edges[k].props;
}
/**
* Group (container) style: ALWAYS transparent (`fillColor=none;container=1;`)
* per the spec, carrying the preset's group stroke/font. `dropTarget=1` marks it
* a drop target in the editor; `verticalAlign=top;align=left;spacingLeft=8;` puts
* the group label in the top-left like draw.io's own boundary containers.
*/
export function groupStyle(preset: PresetData): string {
return (
`rounded=0;whiteSpace=wrap;html=1;` +
`fillColor=none;container=1;dropTarget=1;collapsible=0;` +
`strokeColor=${preset.group.strokeColor};fontColor=${preset.group.fontColor};` +
`verticalAlign=top;align=left;spacingLeft=8;spacingTop=4;`
);
}
+8 -24
View File
@@ -83,32 +83,16 @@ export function filterComment(comment: any, markdownContent?: string) {
};
}
// Map one server search hit to the MCP output contract (#443):
// { pageId, title, path, snippet, score }
//
// INVARIANT: the only page identifier exposed is `pageId` (the server `id`
// UUID). The server also carries `slugId` — it is NEVER surfaced.
//
// GRACEFUL DEGRADATION: against a stock upstream server the opt-in lookup DTO
// fields are stripped, so the response is the legacy FTS shape (no path/snippet/
// score, a `highlight` + `rank` instead). We synthesize the contract from
// whatever is present: `snippet` falls back to the FTS `highlight`, `score` to
// the FTS `rank`, and `path` to [] (upstream has no path). This keeps the tool
// usable even when the server has not been upgraded.
export function filterSearchResult(result: any) {
return {
pageId: result.id,
id: result.id,
title: result.title,
path: Array.isArray(result.path) ? result.path : [],
snippet:
typeof result.snippet === "string"
? result.snippet
: (result.highlight ?? ""),
score:
typeof result.score === "number"
? result.score
: typeof result.rank === "number"
? result.rank
: 0,
parentPageId: result.parentPageId,
createdAt: result.createdAt,
updatedAt: result.updatedAt,
rank: result.rank,
highlight: result.highlight,
spaceId: result.space?.id,
spaceName: result.space?.name,
};
}
+8 -85
View File
@@ -1,30 +1,11 @@
/**
* Options for `buildPageTree`. Fully OPTIONAL so the existing call form
* `buildPageTree(nodes)` keeps its historic behaviour (lean `{id, slugId,
* title, children?}` output, no depth cut) unchanged.
*
* - `shape: "getTree"` emit the #443 `getTree` output node shape
* `{pageId, title, children?, hasChildren?}` instead of the lean
* `{id, slugId, title, children?}` shape. `slugId`/`icon`/`position` are
* never exposed (INVARIANT: only the UUID `pageId` leaves the MCP layer).
* - `maxDepth` trim the built tree to this many levels (root nodes are
* depth 1). Only meaningful together with `shape: "getTree"` (the lean shape
* has no `hasChildren` to signal a cut). See the depth logic below.
*/
export interface BuildPageTreeOptions {
shape?: "lean" | "getTree";
maxDepth?: number;
}
/**
* Pure tree-builder: turn a flat array of sidebar-style page nodes (as produced
* by `enumerateSpacePages`) into a nested tree.
*
* Input: a flat array of nodes. Each node is expected to carry at least
* { id, slugId, title, position, parentPageId } (extra fields are ignored),
* plus a server `hasChildren` boolean used by the `getTree` shape below.
* { id, slugId, title, position, parentPageId } (extra fields are ignored).
*
* Output (default / `shape: "lean"`): an array of ROOT nodes, each shaped as
* Output: an array of ROOT nodes, each shaped as
* { id, slugId, title, children? }
* where `children` is the array of child nodes (same shape, recursively). The
* `children` key is OMITTED entirely when a node has no children consistent
@@ -32,14 +13,6 @@ export interface BuildPageTreeOptions {
* lean (nesting alone conveys the structure; parentPageId/position/hasChildren
* are intentionally dropped from the output).
*
* Output (`shape: "getTree"`, the #443 tool shape): each node is
* { pageId, title, children?, hasChildren? }
* the server `id` is exposed as `pageId` (never `slugId`/`icon`/`position`).
* `children` is omitted for leaves and for nodes trimmed by `maxDepth`.
* `hasChildren: true` is set ONLY on a node whose children exist on the server
* (per the flat item's `hasChildren`) but were CUT by `maxDepth`; on leaves and
* on fully-expanded interior nodes the field is omitted (see `maxDepth` below).
*
* Linking rule: a node is attached as a child of `parentPageId` only when that
* parent id is actually present in the input. Otherwise including a null /
* undefined `parentPageId`, or a parent that was capped out of the bounded walk
@@ -53,42 +26,18 @@ export interface BuildPageTreeOptions {
* fractional-index ASCII keys (e.g. "a0", "a1"). Nodes with a missing/undefined
* `position` sort last.
*
* maxDepth (getTree shape only): the tree is built in FULL first, then trimmed
* on the way out. Root nodes are depth 1. `maxDepth: N` keeps nodes at depth
* <= N and drops the `children` of any node AT depth N. A node whose children
* were dropped this way gets `hasChildren: true` when it actually had children
* in the flat input (source of truth = the server `hasChildren` flag), so the
* caller knows it can descend further with a follow-up `rootPageId` call. An
* absent/undefined `maxDepth` means no cut (whole tree). `maxDepth <= 0` is
* treated as "no cut" (defensive; the tool schema clamps to >= 1).
*
* Pure: no I/O, no network, deterministic.
*/
export function buildPageTree(
nodes: any[],
options: BuildPageTreeOptions = {},
): any[] {
const getTreeShape = options.shape === "getTree";
// A finite, positive cut only; anything else means "no cut".
const maxDepth =
typeof options.maxDepth === "number" &&
Number.isFinite(options.maxDepth) &&
options.maxDepth > 0
? Math.floor(options.maxDepth)
: undefined;
type InternalNode = {
export function buildPageTree(nodes: any[]): any[] {
type OutputNode = {
id: string;
// Retained internally for shaping; never all emitted at once.
slugId: any;
title: any;
hasServerChildren: boolean;
children?: InternalNode[];
children?: OutputNode[];
};
// Map id -> internal node. Build up front; the output shape is projected at
// the very end so the maxDepth cut can consult `hasServerChildren`.
const byId = new Map<string, InternalNode>();
// Map id -> output node. Build the lean output shape up front.
const byId = new Map<string, OutputNode>();
// Preserve the original position string for sorting (kept off the output).
const positionById = new Map<string, string | undefined>();
@@ -100,7 +49,6 @@ export function buildPageTree(
id: node.id,
slugId: node.slugId,
title: node.title,
hasServerChildren: node.hasChildren === true,
});
positionById.set(node.id, node.position);
}
@@ -142,30 +90,5 @@ export function buildPageTree(
}
roots.sort(byPosition);
const rootNodes = roots.map((id) => byId.get(id)!);
// Project the internal nodes into the requested OUTPUT shape, applying the
// maxDepth cut for the getTree shape. `depth` is 1-based (roots = depth 1).
const project = (node: InternalNode, depth: number): any => {
if (getTreeShape) {
const out: any = { pageId: node.id, title: node.title };
const atCut = maxDepth !== undefined && depth >= maxDepth;
if (!atCut && node.children && node.children.length > 0) {
out.children = node.children.map((c) => project(c, depth + 1));
} else if (atCut && node.hasServerChildren) {
// Children exist on the server but were trimmed by maxDepth: signal it
// so the caller can descend with a follow-up rootPageId call.
out.hasChildren = true;
}
return out;
}
// Lean (historic) shape: cycle-safe, no depth cut, no hasChildren.
const out: any = { id: node.id, slugId: node.slugId, title: node.title };
if (node.children && node.children.length > 0) {
out.children = node.children.map((c) => project(c, depth + 1));
}
return out;
};
return rootNodes.map((n) => project(n, 1));
return roots.map((id) => byId.get(id)!);
}
+3 -8
View File
@@ -40,8 +40,8 @@ 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 by a fragment of a technical string (hostname/IP/ID like srv.local, 10.0.12, WB-MGE-30D86B) -> search — hybrid substring + full-text, returns each hit's location (path: root->parent titles) and a snippet around the match, so you rarely need a follow-up getPage; scope with spaceId or parentPageId (a subtree), titleOnly to match titles only. A space's page HIERARCHY (or one subtree) -> getTree (one request, complete, `{pageId,title,children?}`; rootPageId for a subtree, maxDepth to trim depth — a trimmed node gets hasChildren:true); prefer it over listPages tree:true (deprecated). Have a pageId, need WHERE-AM-I / what's around it (its breadcrumbs + direct children, metadata only) -> getPageContext (one call; parent = last breadcrumb, [] for a root page). list -> listPages / listSpaces. Locate blocks and their ids CHEAPLY -> getOutline (compact top-level map; start here, not getPageJson). One block, for editing -> getNode (by attrs.id, or \"#<index>\" for tables, which carry no id) — returns MARKDOWN by default (comment anchors kept for safe write-back); pass format:\"json\" for the raw ProseMirror subtree. Find every occurrence of a string/regex ON a page (and where each is) -> searchInPage, NOT block-by-block getNode — it returns each hit's node ref + block index + context for a targeted comment. Whole page -> getPage (Markdown, canonical for text; drops only block ids, resolved-comment anchors, and a fixed no-md-representation attr set: table spans/colwidth/bg, indent, callout.icon, orderedList.type, link internal/target/rel/class; inline <span data-comment-id> tags are comment anchors — markup, not text) or getPageJson (full ProseMirror with block ids, for those dropped attrs). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stashPage (returns a short-lived anonymous URL).\n" +
"EDIT: fix wording/typos/numbers -> editPageText (find/replace inside blocks, no node id needed). Edit a block -> getNode(markdown) -> edit the markdown -> patchNode(markdown) (by attrs.id from getOutline; the markdown fragment may be several blocks — a 1->N section rewrite in one call, the first block keeps the id). Reach for patchNode's `node`-JSON only for fine attr/mark work; a table cell with spans/colors/fixed width -> the table tools (patchNode markdown refuses it). Add a block -> insertNode (markdown, before/after a block by attrs.id or by anchor text, or append; `node` for raw JSON or bare table structure). Remove a block -> deleteNode (by attrs.id). Tables -> tableGet / tableUpdateCell / tableInsertRow / tableDeleteRow (address by \"#<index>\" from getOutline; table nodes have no attrs.id). Images -> insertImage (add from a web URL) / replaceImage (swap an existing image). Draw.io diagrams -> PREFER the high-level semantic tools that hide coordinates/styles: drawioFromGraph (architecture/cloud/network diagrams — describe nodes/groups/edges by kind+icon, the server picks layout, colors and verified icons; hints layer/sameLayerAs/pinned and layout:full|incremental|none) and drawioFromMermaid (standard flowcharts — write Mermaid, get an editable diagram). For targeted tweaks of an existing diagram use drawioEditCells (id-based add/update/delete with cascade delete + baseHash lock). Raw mxGraph XML via drawioCreate/drawioUpdate is the escape-hatch for exotic/wireframe diagrams; drawioGet reads a diagram as mxGraph XML + a hash (pass it as baseHash to drawioUpdate/drawioEditCells for optimistic locking). Before authoring raw XML, drawioShapes (look up verified stencil style-strings so a shape name never renders as an empty box) and drawioGuide (on-demand authoring reference: skeleton/layout/containers/icons-aws/icons-azure), and pass layout:\"elk\" to drawioCreate/drawioUpdate to auto-place nodes. Footnotes -> insertFootnote. Bulk/structural rewrite -> updatePageJson (full ProseMirror replace) or updatePageMarkdown (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) -> docmostTransform: 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" +
"READ: find a page -> search (workspace-wide full-text); list -> listPages / listSpaces. Locate blocks and their ids CHEAPLY -> getOutline (compact top-level map; start here, not getPageJson). One block, for editing -> getNode (by attrs.id, or \"#<index>\" for tables, which carry no id) — returns MARKDOWN by default (comment anchors kept for safe write-back); pass format:\"json\" for the raw ProseMirror subtree. Find every occurrence of a string/regex ON a page (and where each is) -> searchInPage, NOT block-by-block getNode — it returns each hit's node ref + block index + context for a targeted comment. Whole page -> getPage (Markdown, canonical for text; drops only block ids, resolved-comment anchors, and a fixed no-md-representation attr set: table spans/colwidth/bg, indent, callout.icon, orderedList.type, link internal/target/rel/class; inline <span data-comment-id> tags are comment anchors — markup, not text) or getPageJson (full ProseMirror with block ids, for those dropped attrs). Hand a huge page (with images) to an external consumer without pulling it through the model context -> stashPage (returns a short-lived anonymous URL).\n" +
"EDIT: fix wording/typos/numbers -> editPageText (find/replace inside blocks, no node id needed). Edit a block -> getNode(markdown) -> edit the markdown -> patchNode(markdown) (by attrs.id from getOutline; the markdown fragment may be several blocks — a 1->N section rewrite in one call, the first block keeps the id). Reach for patchNode's `node`-JSON only for fine attr/mark work; a table cell with spans/colors/fixed width -> the table tools (patchNode markdown refuses it). Add a block -> insertNode (markdown, before/after a block by attrs.id or by anchor text, or append; `node` for raw JSON or bare table structure). Remove a block -> deleteNode (by attrs.id). Tables -> tableGet / tableUpdateCell / tableInsertRow / tableDeleteRow (address by \"#<index>\" from getOutline; table nodes have no attrs.id). Images -> insertImage (add from a web URL) / replaceImage (swap an existing image). Draw.io diagrams -> drawioCreate (create from mxGraph XML and insert), drawioGet (read a diagram as mxGraph XML + a hash), drawioUpdate (replace a diagram; pass the hash from drawioGet as baseHash for optimistic locking); before authoring a diagram, drawioShapes (look up verified stencil style-strings so a shape name never renders as an empty box) and drawioGuide (on-demand authoring reference: skeleton/layout/containers/icons-aws/icons-azure), and pass layout:\"elk\" to drawioCreate/drawioUpdate to auto-place nodes. Footnotes -> insertFootnote. Bulk/structural rewrite -> updatePageJson (full ProseMirror replace) or updatePageMarkdown (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) -> docmostTransform: 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 -> createPage (Markdown). Rename (title only) -> renamePage. Move -> movePage. Delete -> deletePage (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) -> copyPageContent. Sharing -> sharePage / unsharePage / listShares; sharePage makes the page PUBLICLY accessible — do it only when explicitly asked.\n" +
"COMMENTS: createComment 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 -> createComment 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 -> listComments, updateComment, resolveComment (resolve/reopen, reversible — prefer over delete to close), deleteComment, checkNewComments.\n" +
"HISTORY: review what changed -> diffPageVersions (a historyId vs current, or two versions). List saved versions -> listPageHistory. Undo a bad edit -> restorePageVersion (writes a past version back as current; itself revertible). Export a page to self-contained Docmost Markdown (with comment anchors) -> exportPageMarkdown.";
@@ -82,8 +82,6 @@ const TOOL_FAMILY: Record<string, Family> = {
// READ
search: "READ",
listPages: "READ",
getTree: "READ",
getPageContext: "READ",
listSpaces: "READ",
getOutline: "READ",
getNode: "READ",
@@ -109,9 +107,6 @@ const TOOL_FAMILY: Record<string, Family> = {
drawioGet: "EDIT",
drawioCreate: "EDIT",
drawioUpdate: "EDIT",
drawioEditCells: "EDIT",
drawioFromGraph: "EDIT",
drawioFromMermaid: "EDIT",
drawioShapes: "EDIT",
drawioGuide: "EDIT",
docmostTransform: "EDIT",
@@ -157,7 +152,7 @@ export const INLINE_MCP_INVENTORY: ToolInventoryLine[] = [
{
name: "search",
purpose:
"find pages by a fragment of a technical string (hybrid substring + full-text); returns each hit's path and a snippet.",
"full-text search for pages and content across the whole workspace.",
},
{
name: "docmostTransform",
+4 -312
View File
@@ -63,8 +63,6 @@ export type DocmostClientLike = Pick<
| 'getSpaces'
| 'listShares'
| 'listPages'
| 'getTree'
| 'getPageContext'
| 'getPage'
| 'getPageJson'
| 'getOutline'
@@ -100,9 +98,6 @@ export type DocmostClientLike = Pick<
| 'drawioGet'
| 'drawioCreate'
| 'drawioUpdate'
| 'drawioEditCells'
| 'drawioFromGraph'
| 'drawioFromMermaid'
| 'createComment'
| 'resolveComment'
>;
@@ -922,13 +917,11 @@ export const SHARED_TOOL_SPECS = {
description:
'List the most recent pages (ordered by updatedAt, descending), ' +
'optionally scoped to a single space. Returns a bounded list (default ' +
'50, max 100) — use search for lookups in large spaces. tree:true (with ' +
"spaceId) returns the space's full page hierarchy as a nested tree, but " +
'is DEPRECATED — use getTree instead (leaner nodes, plus rootPageId / ' +
'maxDepth).',
'50, max 100) — use search for lookups in large spaces. Pass tree:true ' +
"(with spaceId) to instead get the space's full page hierarchy as a " +
'nested tree.',
tier: 'core',
catalogLine:
"listPages — list recent pages (tree:true is deprecated; use getTree for the hierarchy).",
catalogLine: "listPages — list recent pages, or a space's full page tree.",
buildShape: (z) => ({
spaceId: z
.string()
@@ -961,79 +954,6 @@ export const SHARED_TOOL_SPECS = {
),
},
getTree: {
mcpName: 'getTree',
inAppKey: 'getTree',
description:
"Get a space's page hierarchy (or one subtree) as a nested tree in a " +
'SINGLE request — completely and without loss. Each node is ' +
'`{ pageId, title, children? }`; children are ordered as in the sidebar. ' +
'Pass rootPageId to return only that page and its descendants (exactly ' +
'one root). Pass maxDepth to trim depth and save tokens (root nodes are ' +
'depth 1, so maxDepth:1 returns only the roots); a node whose children ' +
'were trimmed carries `hasChildren:true` so you can descend later with ' +
'getTree(rootPageId=that page). Prefer this over listPages tree:true.',
tier: 'core',
catalogLine:
"getTree — a space's page hierarchy (or a subtree) as a nested tree in one request.",
buildShape: (z) => ({
spaceId: z
.string()
.min(1)
.describe('The id of the space whose page tree to return.'),
rootPageId: z
.string()
.optional()
.describe(
'Optional page id: return only this page and its descendants (one root).',
),
maxDepth: z
.number()
.int()
.min(1)
.optional()
.describe(
'Optional depth cap (roots are depth 1). maxDepth:1 returns only the ' +
'roots; trimmed nodes carry hasChildren:true.',
),
}),
execute: (client, { spaceId, rootPageId, maxDepth }) =>
client.getTree(
spaceId as string,
rootPageId as string | undefined,
maxDepth as number | undefined,
),
},
getPageContext: {
mcpName: 'getPageContext',
inAppKey: 'getPageContext',
description:
'Given a pageId, get its LOCATION and immediate surroundings (metadata ' +
'only, no page content) in one call — answers "where am I / what is ' +
"around this page\". Returns `{ page: { pageId, title, spaceId }, " +
'breadcrumbs: [{ pageId, title }], children: [{ pageId, title, ' +
'hasChildren }] }`. `breadcrumbs` is the ancestor chain from the space ' +
'root down to the PARENT (the parent is its last element; a root page ' +
'has `breadcrumbs: []`). `children` are the direct children in sidebar ' +
'order, each flagged `hasChildren` so you know which can be expanded ' +
'(descend with getTree(rootPageId=that child) or another getPageContext). ' +
'Ids, titles and child order are consistent with getTree.',
tier: 'core',
catalogLine:
'getPageContext — a page’s breadcrumbs + direct children (where-am-I) in one call.',
buildShape: (z) => ({
pageId: z
.string()
.min(1)
.describe(
'The id of the page to locate (a pageId/UUID, or a slugId from a URL).',
),
}),
execute: (client, { pageId }) =>
client.getPageContext(pageId as string),
},
createPage: {
mcpName: 'createPage',
inAppKey: 'createPage',
@@ -2016,234 +1936,6 @@ export const SHARED_TOOL_SPECS = {
),
},
drawioEditCells: {
mcpName: 'drawioEditCells',
inAppKey: 'drawioEditCells',
description:
'Make TARGETED, id-based edits to an existing draw.io diagram instead of ' +
'resending the whole XML (a full-XML diff is fragile — draw.io reorders ' +
'attributes). `operations` is an ordered list of: ' +
'{ op:"add", xml:"<mxCell .../>" } (append a new cell), ' +
'{ op:"update", cellId:"n3", xml:"<mxCell id=\\"n3\\" .../>" } (replace that ' +
'cell; the id MUST stay the same), or { op:"delete", cellId:"n5" } — a ' +
'delete CASCADES to the cell\'s container children AND to every edge whose ' +
'source/target is deleted. Ids are STABLE across edits so diffs stay ' +
'meaningful. `baseHash` is MANDATORY: pass the hash from the drawioGet you ' +
'based the edit on; if the diagram changed since, the edit is refused with ' +
'a conflict error — re-read with drawioGet and retry. The edited model goes ' +
'through the same lint + quality-warning pipeline as drawioUpdate. `node` is ' +
'the drawio node attrs.id or "#<index>". Use this to tweak a diagram (move ' +
'or restyle a few cells, add/remove nodes); to (re)generate a whole diagram ' +
'from a description use drawioFromGraph.' +
DRAWIO_HARD_RULES,
tier: 'deferred',
catalogLine:
'drawioEditCells — id-based add/update/delete edits to a draw.io diagram (cascade delete).',
buildShape: (z) => ({
pageId: z.string().min(1),
node: z
.string()
.min(1)
.describe('The drawio node attrs.id, or "#<index>" for a top-level block.'),
operations: z
.array(
z.object({
op: z.enum(['add', 'update', 'delete']),
cellId: z
.string()
.optional()
.describe('Target cell id (required for update/delete).'),
xml: z
.string()
.optional()
.describe('The <mxCell> element (required for add/update).'),
}),
)
.describe('Ordered add/update/delete operations keyed by cell id.'),
baseHash: z
.string()
.min(1)
.describe('The meta.hash from the drawioGet this edit is based on.'),
}),
execute: (client, { pageId, node, operations, baseHash }) =>
client.drawioEditCells(
pageId as string,
node as string,
operations as any,
baseHash as string,
),
},
drawioFromGraph: {
mcpName: 'drawioFromGraph',
inAppKey: 'drawioFromGraph',
description:
'Build a draw.io diagram from a SEMANTIC graph — you describe nodes, groups ' +
'and edges by MEANING and the server picks every coordinate, color and icon ' +
'so the whole class of layout/icon mistakes (overlaps, edges through shapes, ' +
'empty-box stencils) cannot happen. This is the PREFERRED tool for ' +
'architecture / cloud / network diagrams. `graph` = { nodes:[{ id, label, ' +
'kind?, icon?, group?, layer?, sameLayerAs?, pinned? }], groups?:[{ id, ' +
'label, kind? }], edges?:[{ from, to, label?, kind? }] }. Node `kind` picks ' +
'a palette color (service/db/queue/gateway/error/external/security); `icon` ' +
'(e.g. "aws:lambda", "aws:dynamodb", "azure:cosmos") resolves to the exact ' +
'verified stencil — an unknown icon degrades to a labelled generic shape, ' +
'never an empty box. Edge `kind` sets the line style (sync=solid, ' +
'async=dashed, error=red-dashed). Groups are TRANSPARENT containers. ' +
'`direction` (LR/RL/TB/BT) and `preset` (default/dark/colorblind-safe) tune ' +
'the layout/palette. Layout hints: `layer` (column index), `sameLayerAs` ' +
'(align two nodes), `pinned:{x,y}` (fix a node). `layout`: "full" (default, ' +
'auto-place everything), "incremental" (with `node`: keep the existing ' +
'diagram\'s coordinates, place only new cells), "none" (no auto-layout). The ' +
'result reports { iconsResolved, iconsMissing } so you can verify all icons ' +
'resolved. For standard flowcharts you can also write Mermaid and call ' +
'drawioFromMermaid; for exotic/wireframe diagrams use raw XML via drawioCreate.',
tier: 'deferred',
catalogLine:
'drawioFromGraph — build a draw.io diagram from a semantic node/group/edge graph (server picks layout+icons).',
buildShape: (z) => {
const node = z.object({
id: z.string().min(1),
label: z.string().min(1),
kind: z
.string()
.optional()
.describe(
'Palette slot: service/db/queue/gateway/error/external/security.',
),
icon: z
.string()
.optional()
.describe('Icon ref, e.g. "aws:lambda", "aws:dynamodb", "azure:cosmos".'),
group: z.string().optional().describe('Id of the group (container) it sits in.'),
layer: z.number().optional().describe('Layer/column index hint (>=0).'),
sameLayerAs: z
.string()
.optional()
.describe('Put this node in the same layer as another node id.'),
pinned: z
.object({ x: z.number(), y: z.number() })
.optional()
.describe('Fix the node at these exact coordinates.'),
});
const group = z.object({
id: z.string().min(1),
label: z.string().min(1),
kind: z.string().optional(),
});
const edge = z.object({
from: z.string().min(1),
to: z.string().min(1),
label: z.string().optional(),
kind: z
.string()
.optional()
.describe('sync (solid), async (dashed), error (red-dashed).'),
});
return {
pageId: z.string().min(1),
graph: z
.object({
nodes: z.array(node),
groups: z.array(group).optional(),
edges: z.array(edge).optional(),
direction: z.enum(['LR', 'RL', 'TB', 'BT']).optional(),
preset: z.enum(['default', 'dark', 'colorblind-safe']).optional(),
})
.describe('The semantic graph: nodes, groups, edges.'),
position: z
.enum(['before', 'after', 'append'])
.describe('Where to insert relative to the anchor.'),
anchorNodeId: z.string().optional().describe('Anchor block id (for before/after).'),
anchorText: z.string().optional().describe('Anchor text fragment (for before/after).'),
direction: z
.enum(['LR', 'RL', 'TB', 'BT'])
.optional()
.describe('Layout direction (overrides graph.direction).'),
preset: z
.enum(['default', 'dark', 'colorblind-safe'])
.optional()
.describe('Color preset (overrides graph.preset).'),
layout: z
.enum(['none', 'full', 'incremental'])
.optional()
.describe(
'"full" (default) auto-places all; "incremental" (with node) keeps ' +
'existing coords and places only new cells; "none" no auto-layout.',
),
node: z
.string()
.optional()
.describe(
'An existing diagram to (re)build into — required for layout:"incremental".',
),
};
},
execute: (
client,
{ pageId, graph, position, anchorNodeId, anchorText, direction, preset, layout, node },
) =>
client.drawioFromGraph(
pageId as string,
{
position: position as 'before' | 'after' | 'append',
anchorNodeId: anchorNodeId as string | undefined,
anchorText: anchorText as string | undefined,
},
graph as any,
direction as 'LR' | 'RL' | 'TB' | 'BT' | undefined,
preset as string | undefined,
layout as 'none' | 'full' | 'incremental' | undefined,
node as string | undefined,
),
},
drawioFromMermaid: {
mcpName: 'drawioFromMermaid',
inAppKey: 'drawioFromMermaid',
description:
'Convert Mermaid `flowchart` text into an EDITABLE draw.io diagram (LLMs ' +
'write Mermaid reliably). Best for STANDARD flowcharts/decision trees: ' +
'write the mermaid, the server parses it (pure parser — no browser/CLI), ' +
'maps it to the same semantic pipeline as drawioFromGraph, and inserts a ' +
'real draw.io diagram you can then refine with drawioEditCells. Node shapes ' +
'map to palette colors (a `{decision}` -> yellow, a `[(db)]` -> green, etc.); ' +
'`subgraph … end` becomes a transparent group; dotted `-.->` edges become ' +
'dashed. ONLY flowchart/graph is supported — for sequence/class diagrams, or ' +
'for cloud/architecture diagrams with real service icons, use drawioFromGraph ' +
'instead. `where` positions the block like insertNode.',
tier: 'deferred',
catalogLine:
'drawioFromMermaid — turn Mermaid flowchart text into an editable draw.io diagram.',
buildShape: (z) => ({
pageId: z.string().min(1),
mermaid: z
.string()
.min(1)
.describe('Mermaid flowchart source (flowchart/graph LR|TB|...).'),
position: z
.enum(['before', 'after', 'append'])
.describe('Where to insert relative to the anchor.'),
anchorNodeId: z.string().optional().describe('Anchor block id (for before/after).'),
anchorText: z.string().optional().describe('Anchor text fragment (for before/after).'),
preset: z
.enum(['default', 'dark', 'colorblind-safe'])
.optional()
.describe('Color preset.'),
}),
execute: (client, { pageId, mermaid, position, anchorNodeId, anchorText, preset }) =>
client.drawioFromMermaid(
pageId as string,
{
position: position as 'before' | 'after' | 'append',
anchorNodeId: anchorNodeId as string | undefined,
anchorText: anchorText as string | undefined,
},
mermaid as string,
preset as string | undefined,
),
},
drawioShapes: {
mcpName: 'drawioShapes',
inAppKey: 'drawioShapes',
+2 -68
View File
@@ -316,8 +316,7 @@ async function main() {
const [idA, idB, idC] = seedIds;
// patchNode: replace the middle paragraph; siblings' ids must be unchanged.
// #413 XOR input: the raw ProseMirror node goes under the `node` key.
await client.patchNode(nid, idB, { node: mkPara(idB, "Bravo PATCHED.") });
await client.patchNode(nid, idB, mkPara(idB, "Bravo PATCHED."));
await new Promise((r) => setTimeout(r, 16000));
const afterPatch = (await client.getPageJson(nid)).content;
const patchText = JSON.stringify(afterPatch);
@@ -328,7 +327,7 @@ async function main() {
// insertNode: place a new block after the first paragraph.
await client.insertNode(
nid,
{ node: mkPara("nodeops-ins", "Inserted paragraph.") },
mkPara("nodeops-ins", "Inserted paragraph."),
{ position: "after", anchorNodeId: idA },
);
await new Promise((r) => setTimeout(r, 16000));
@@ -434,71 +433,6 @@ async function main() {
}
}
// 6h. markdown converter fixpoint (#476): pins the converter fixpoint
// THROUGH the live server/collab path, not just the package tests. The
// unit corpus (docmost-md-roundtrip) proves the converter alone is a
// fixpoint; this asserts the property survives the real pipeline — export
// (REST read, PM -> MD) -> import (MD -> PM -> collab replace -> server
// persistence) -> export — where the server schema, the Yjs structural
// diff or the collab write path could still mangle the doc while every
// unit test stays green. importPageMarkdown is the designed inverse of
// exportPageMarkdown (the self-contained envelope with meta/comments
// blocks); updatePageMarkdown (client.updatePage) takes plain authoring
// markdown and would re-import the envelope blocks as literal content.
{
const FIXMD = [
"# Fixpoint heading",
"",
"Paragraph with **bold**, *italic* and a [link](https://example.com).",
"",
"## Second level",
"",
"- bullet one",
"- bullet two",
"",
"1. ordered one",
"2. ordered two",
"",
"```js",
"const answer = 42; // code block must survive byte-identically",
"```",
"",
"| A | B |",
"| --- | --- |",
"| one | two |",
"",
":::info",
"Callout body.",
":::",
].join("\n");
const fx = await client.createPage("E2E md fixpoint " + Date.now(), FIXMD, spaceId);
const fxid = fx.data.id;
try {
const md1 = await client.exportPageMarkdown(fxid);
await client.importPageMarkdown(fxid, md1);
await new Promise((r) => setTimeout(r, 16000)); // wait for server persistence
const md2 = await client.exportPageMarkdown(fxid);
// On failure, name the first diverging line of the two exports.
const firstDiff = (a, b) => {
const al = a.split("\n");
const bl = b.split("\n");
for (let i = 0; i < Math.max(al.length, bl.length); i++) {
if (al[i] !== bl[i]) {
return `first diff at line ${i + 1}: ${JSON.stringify(al[i] ?? "<EOF>")} -> ${JSON.stringify(bl[i] ?? "<EOF>")}`;
}
}
return "same lines, different bytes (line endings?)";
};
check(
"markdown fixpoint: export -> import -> export is byte-identical",
md1 === md2,
md1 === md2 ? "" : firstDiff(md1, md2),
);
} finally {
try { await client.deletePage(fxid); } catch {}
}
}
// 7. shares: create (idempotent), public access, list, unshare
const share = await client.sharePage(pageId);
check("sharePage: returns public URL", share.publicUrl?.startsWith(`${APP}/share/`), share.publicUrl);
@@ -1,272 +0,0 @@
// Contract tests for the stage-3 drawio client methods (issue #425):
// drawioEditCells / drawioFromGraph / drawioFromMermaid. Same seam-override
// pattern as drawio-tools.test.mjs: a DocmostClient subclass stubs the I/O seams
// so the tool logic runs without a live Docmost / collab socket.
import { test } from "node:test";
import assert from "node:assert/strict";
import { DocmostClient } from "../../build/client.js";
import {
buildDrawioSvg,
normalizeXml,
mxHash,
decodeDrawioSvg,
parseCells,
} from "../../build/lib/drawio-xml.js";
const DRAWIO_SCHEMA_ATTRS = new Set([
"src", "title", "alt", "width", "height", "size", "aspectRatio", "align", "attachmentId",
]);
function applyDrawioSchemaDrop(node) {
if (!node || typeof node !== "object") return;
if (node.type === "drawio" && node.attrs && typeof node.attrs === "object") {
for (const key of Object.keys(node.attrs))
if (!DRAWIO_SCHEMA_ATTRS.has(key)) delete node.attrs[key];
}
if (Array.isArray(node.content)) for (const c of node.content) applyDrawioSchemaDrop(c);
}
function svgFor(model, bbox = { width: 400, height: 300 }) {
return buildDrawioSvg(normalizeXml(model), "<g/>", bbox);
}
function makeClient({ pageDoc, attachmentSvg } = {}) {
const calls = { uploads: [], mutations: [] };
class TestClient extends DocmostClient {
async ensureAuthenticated() {}
async getCollabTokenWithReauth() {
return "collab-token";
}
async resolvePageId(pageId) {
return `uuid-${pageId}`;
}
async getPageRaw(pageId) {
return {
id: pageId, slugId: "s", title: "P", spaceId: "sp",
content: pageDoc ?? { type: "doc", content: [] },
};
}
async uploadAttachmentBuffer(pageId, buffer, fileName) {
const id = `att-${calls.uploads.length + 1}`;
calls.uploads.push({ pageId, fileName, svg: buffer.toString("utf-8") });
return { id, fileName, fileSize: buffer.length };
}
async fetchAttachmentText() {
return attachmentSvg;
}
mutatePage(pageId, token, apiUrl, transform) {
const clone = structuredClone(pageDoc ?? { type: "doc", content: [] });
const doc = transform(clone);
if (doc) applyDrawioSchemaDrop(doc);
calls.mutations.push({ pageId, doc });
return Promise.resolve({ doc, verify: { changed: doc != null } });
}
}
const client = new TestClient("http://127.0.0.1:1/api", "e@x.com", "pw");
return { client, calls };
}
function findDrawio(node, acc = []) {
if (!node || typeof node !== "object") return acc;
if (node.type === "drawio") acc.push(node);
if (Array.isArray(node.content)) for (const c of node.content) findDrawio(c, acc);
return acc;
}
// A stored diagram: a group with two children and an edge.
const STORED =
"<mxGraphModel><root><mxCell id=\"0\"/><mxCell id=\"1\" parent=\"0\"/>" +
'<mxCell id="grp" value="G" style="container=1;fillColor=none;" vertex="1" parent="1">' +
'<mxGeometry x="0" y="0" width="300" height="200" as="geometry"/></mxCell>' +
'<mxCell id="a" value="A" style="rounded=1;" vertex="1" parent="grp">' +
'<mxGeometry x="10" y="10" width="80" height="40" as="geometry"/></mxCell>' +
'<mxCell id="b" value="B" style="rounded=1;" vertex="1" parent="grp">' +
'<mxGeometry x="10" y="90" width="80" height="40" as="geometry"/></mxCell>' +
'<mxCell id="e" style="" edge="1" parent="grp" source="a" target="b">' +
'<mxGeometry relative="1" as="geometry"/></mxCell>' +
"</root></mxGraphModel>";
function drawioPageDoc() {
return {
type: "doc",
content: [
{
type: "drawio",
attrs: {
id: "d1", src: "/api/files/att-1/diagram.drawio.svg",
attachmentId: "att-1", width: 400, height: 300,
},
},
],
};
}
// --- drawioEditCells --------------------------------------------------------
test("drawioEditCells: applies ops and repoints the node (current baseHash)", async () => {
const { client, calls } = makeClient({
pageDoc: drawioPageDoc(),
attachmentSvg: svgFor(STORED),
});
const baseHash = mxHash(normalizeXml(STORED));
const res = await client.drawioEditCells(
"page1",
"d1",
[
{
op: "update",
cellId: "a",
xml:
'<mxCell id="a" value="Renamed" style="rounded=1;" vertex="1" parent="grp">' +
'<mxGeometry x="10" y="10" width="80" height="40" as="geometry"/></mxCell>',
},
],
baseHash,
);
assert.equal(res.success, true);
assert.equal(calls.uploads.length, 1);
const written = decodeDrawioSvg(calls.uploads[0].svg);
const cells = parseCells(written);
assert.equal(cells.find((c) => c.id === "a").value, "Renamed");
assert.equal(cells.find((c) => c.id === "b").value, "B"); // untouched
const n = findDrawio(calls.mutations[0].doc)[0];
// The stub numbers uploads from 1; this edit is the first upload -> att-1.
assert.equal(n.attrs.attachmentId, "att-1");
});
test("drawioEditCells: delete of the container cascades to children + edge", async () => {
const { client, calls } = makeClient({
pageDoc: drawioPageDoc(),
attachmentSvg: svgFor(STORED),
});
const baseHash = mxHash(normalizeXml(STORED));
await client.drawioEditCells("page1", "d1", [{ op: "delete", cellId: "grp" }], baseHash);
const written = decodeDrawioSvg(calls.uploads[0].svg);
const ids = parseCells(written).filter((c) => c.id !== "0" && c.id !== "1").map((c) => c.id);
assert.deepEqual(ids, [], "grp + a + b + edge all cascaded away");
});
test("drawioEditCells: stale baseHash -> conflict, no upload", async () => {
const { client, calls } = makeClient({
pageDoc: drawioPageDoc(),
attachmentSvg: svgFor(STORED),
});
await assert.rejects(
() => client.drawioEditCells("page1", "d1", [{ op: "delete", cellId: "a" }], "stale"),
/conflict/,
);
assert.equal(calls.uploads.length, 0);
});
test("drawioEditCells: baseHash is mandatory", async () => {
const { client } = makeClient({ pageDoc: drawioPageDoc(), attachmentSvg: svgFor(STORED) });
await assert.rejects(
() => client.drawioEditCells("page1", "d1", [{ op: "delete", cellId: "a" }], ""),
/baseHash is mandatory/,
);
});
// --- drawioFromGraph --------------------------------------------------------
test("drawioFromGraph: builds a diagram from a graph and inserts a node", async () => {
const pageDoc = { type: "doc", content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }] };
const { client, calls } = makeClient({ pageDoc });
const res = await client.drawioFromGraph(
"page1",
{ position: "append" },
{
nodes: [
{ id: "api", label: "API", kind: "gateway", icon: "aws:api_gateway", group: "vpc" },
{ id: "fn", label: "Handler", kind: "service", icon: "aws:lambda", group: "vpc" },
{ id: "db", label: "Orders", kind: "db", icon: "aws:dynamodb" },
],
groups: [{ id: "vpc", label: "VPC" }],
edges: [{ from: "api", to: "fn", kind: "sync" }, { from: "fn", to: "db", kind: "async" }],
},
"LR",
"default",
);
assert.equal(res.success, true);
assert.equal(res.nodeId, "#1");
assert.equal(res.iconsMissing.length, 0, `unresolved: ${res.iconsMissing}`);
assert.equal(res.iconsResolved, 3);
// The uploaded model decodes back and carries the group + nodes.
const written = decodeDrawioSvg(calls.uploads[0].svg);
const cells = parseCells(written);
assert.ok(cells.some((c) => c.id === "vpc"));
assert.ok(cells.some((c) => c.id === "api"));
// Group is transparent.
const vpc = cells.find((c) => c.id === "vpc");
assert.equal(vpc.styleMap.fillColor, "none");
assert.equal(vpc.styleMap.container, "1");
});
test("drawioFromGraph: an invalid graph throws before any upload", async () => {
const { client, calls } = makeClient({ pageDoc: { type: "doc", content: [] } });
await assert.rejects(
() => client.drawioFromGraph("page1", { position: "append" }, { nodes: [] }),
/non-empty/,
);
assert.equal(calls.uploads.length, 0);
});
test("drawioFromGraph incremental into an existing node keeps prior coords", async () => {
// The stored diagram has a,b at known coords; add a new node c incrementally.
const { client, calls } = makeClient({
pageDoc: drawioPageDoc(),
attachmentSvg: svgFor(STORED),
});
const res = await client.drawioFromGraph(
"page1",
{ position: "append" },
{
nodes: [
{ id: "a", label: "A" },
{ id: "b", label: "B" },
{ id: "c", label: "C new" },
],
edges: [{ from: "b", to: "c" }],
},
undefined,
undefined,
"incremental",
"d1", // target the existing diagram
);
assert.equal(res.success, true);
const written = decodeDrawioSvg(calls.uploads[0].svg);
const cells = parseCells(written);
const a = cells.find((c) => c.id === "a");
const b = cells.find((c) => c.id === "b");
// Existing coords preserved (the stored a/b absolute coords from STORED).
assert.equal(a.geometry.x, 10);
assert.equal(a.geometry.y, 10);
assert.equal(b.geometry.x, 10);
assert.equal(b.geometry.y, 90);
assert.ok(cells.some((c) => c.id === "c"), "new node c added");
});
// --- drawioFromMermaid ------------------------------------------------------
test("drawioFromMermaid: converts a flowchart and inserts a diagram", async () => {
const pageDoc = { type: "doc", content: [{ type: "paragraph", attrs: { id: "p1" }, content: [] }] };
const { client, calls } = makeClient({ pageDoc });
const res = await client.drawioFromMermaid(
"page1",
{ position: "append" },
"flowchart LR\n A[Start] --> B{Choose}\n B -->|yes| C[Done]\n B -->|no| D[Stop]",
);
assert.equal(res.success, true);
const written = decodeDrawioSvg(calls.uploads[0].svg);
const cells = parseCells(written);
for (const id of ["A", "B", "C", "D"]) {
assert.ok(cells.some((c) => c.id === id), `node ${id} present`);
}
});
test("drawioFromMermaid: a non-flowchart is rejected, no upload", async () => {
const { client, calls } = makeClient({ pageDoc: { type: "doc", content: [] } });
await assert.rejects(
() => client.drawioFromMermaid("page1", { position: "append" }, "sequenceDiagram\n A->>B: x"),
/only 'flowchart'\/'graph' is supported/,
);
assert.equal(calls.uploads.length, 0);
});
@@ -1,375 +0,0 @@
// Mock-HTTP tests for DocmostClient.getPageContext — the #443 "where am I /
// what's around" read tool. A local http.createServer stands in for Docmost
// (same harness style as pagination-cursor.test.mjs) so everything is
// deterministic and offline.
//
// Contract pinned here:
// - Two requests: POST /pages/breadcrumbs (ancestor chain root->page, page
// INCLUDED as the LAST element) + listSidebarPages (direct children).
// - Split: last chain element -> `page`; the rest (root->parent) ->
// `breadcrumbs`. A ROOT page (chain length 1) -> breadcrumbs: [].
// - children: {pageId, title, hasChildren} in sidebar order.
// - INVARIANT: only the UUID `pageId` is exposed, never `slugId`.
// - A slugId input is resolved via /pages/info first (adds one request); a
// UUID input short-circuits (stays at two requests).
// - A bad/inaccessible pageId throws a CLEAR error, not an empty object.
import { test, after } from "node:test";
import assert from "node:assert/strict";
import http from "node:http";
import { DocmostClient } from "../../build/client.js";
function readBody(req) {
return new Promise((resolve) => {
let raw = "";
req.on("data", (chunk) => {
raw += chunk;
});
req.on("end", () => resolve(raw));
});
}
function startServer(handler) {
return new Promise((resolve) => {
const server = http.createServer(handler);
server.listen(0, "127.0.0.1", () => {
const { port } = server.address();
resolve({ server, baseURL: `http://127.0.0.1:${port}/api` });
});
});
}
function closeServer(server) {
return new Promise((resolve) => server.close(resolve));
}
function sendJson(res, status, obj, extraHeaders = {}) {
res.writeHead(status, { "Content-Type": "application/json", ...extraHeaders });
res.end(JSON.stringify(obj));
}
const openServers = [];
async function spawn(handler) {
const { server, baseURL } = await startServer(handler);
openServers.push(server);
return { server, baseURL };
}
after(async () => {
await Promise.all(openServers.map((s) => closeServer(s)));
});
function handleLogin(req, res) {
if (req.url === "/api/auth/login") {
sendJson(res, 200, { success: true }, {
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
});
return true;
}
return false;
}
// Two real UUIDs so resolvePageId short-circuits (no /pages/info round-trip).
const ROOT_UUID = "00000000-0000-4000-8000-000000000001";
const MID_UUID = "00000000-0000-4000-8000-000000000002";
const PAGE_UUID = "00000000-0000-4000-8000-000000000003";
const CHILD_A = "00000000-0000-4000-8000-00000000000a";
const CHILD_B = "00000000-0000-4000-8000-00000000000b";
// Build a breadcrumbs response as the server sends it: root->page order, page
// LAST, wrapped in the {data,success} envelope. slugId/icon/position are present
// on the wire (they must NOT leak into the tool output).
function breadcrumbsEnvelope(chain) {
return { success: true, data: chain };
}
// -----------------------------------------------------------------------------
// 1) 3rd-level page: page = last chain element; breadcrumbs = the two ancestors
// root->parent; children mapped {pageId,title,hasChildren} in order; no leak;
// exactly two requests for a UUID input.
// -----------------------------------------------------------------------------
test("getPageContext: 3rd-level page splits chain, maps children, no slugId leak, 2 requests", async () => {
let breadcrumbReqs = 0;
let sidebarReqs = 0;
let infoReqs = 0;
let breadcrumbBody = null;
const { baseURL } = await spawn(async (req, res) => {
const raw = await readBody(req);
if (handleLogin(req, res)) return;
if (req.url === "/api/pages/info") {
infoReqs++;
sendJson(res, 404, {});
return;
}
if (req.url === "/api/pages/breadcrumbs") {
breadcrumbReqs++;
breadcrumbBody = JSON.parse(raw || "{}");
// root -> parent -> page (page LAST). slugId/icon/position on the wire.
sendJson(
res,
200,
breadcrumbsEnvelope([
{ id: ROOT_UUID, slugId: "rootSlug", title: "Infrastructure", spaceId: "sp1", position: "a", icon: null, parentPageId: null, hasChildren: true },
{ id: MID_UUID, slugId: "midSlug", title: "Datacenter A", spaceId: "sp1", position: "a", icon: null, parentPageId: ROOT_UUID, hasChildren: true },
{ id: PAGE_UUID, slugId: "pageSlug", title: "Rack 12", spaceId: "sp1", position: "b", icon: null, parentPageId: MID_UUID, hasChildren: true },
]),
);
return;
}
if (req.url === "/api/pages/sidebar-pages") {
sidebarReqs++;
const body = JSON.parse(raw || "{}");
assert.equal(body.pageId, PAGE_UUID, "children scoped to the page UUID");
assert.equal(body.spaceId, "sp1", "children scoped to the page's space");
sendJson(res, 200, {
success: true,
data: {
items: [
{ id: CHILD_A, slugId: "aSlug", title: "Servers", parentPageId: PAGE_UUID, hasChildren: true, position: "a" },
{ id: CHILD_B, slugId: "bSlug", title: "Network", parentPageId: PAGE_UUID, hasChildren: false, position: "b" },
],
meta: { hasNextPage: false, nextCursor: null },
},
});
return;
}
sendJson(res, 404, {});
});
const client = new DocmostClient(baseURL, "user@example.com", "pw");
const result = await client.getPageContext(PAGE_UUID);
assert.equal(infoReqs, 0, "UUID input short-circuits resolvePageId (no /pages/info)");
assert.equal(breadcrumbReqs, 1, "exactly one breadcrumbs request");
assert.equal(sidebarReqs, 1, "exactly one sidebar request");
assert.deepEqual(breadcrumbBody, { pageId: PAGE_UUID }, "breadcrumbs posts the UUID");
// page = the LAST chain element.
assert.deepEqual(result.page, {
pageId: PAGE_UUID,
title: "Rack 12",
spaceId: "sp1",
});
// breadcrumbs = root->parent (the chain minus the page itself).
assert.deepEqual(result.breadcrumbs, [
{ pageId: ROOT_UUID, title: "Infrastructure" },
{ pageId: MID_UUID, title: "Datacenter A" },
]);
// children mapped in order, hasChildren coerced to boolean.
assert.deepEqual(result.children, [
{ pageId: CHILD_A, title: "Servers", hasChildren: true },
{ pageId: CHILD_B, title: "Network", hasChildren: false },
]);
// No slugId anywhere in the output.
const dump = JSON.stringify(result);
assert.ok(!dump.includes("Slug"), "no slugId leaks into the output");
assert.ok(!/\bslugId\b/.test(dump), "no slugId key in the output");
});
// -----------------------------------------------------------------------------
// 2) ROOT page: chain has ONE element (the page itself) -> breadcrumbs: [].
// -----------------------------------------------------------------------------
test("getPageContext: a root page has breadcrumbs: []", async () => {
const { baseURL } = await spawn(async (req, res) => {
const raw = await readBody(req);
if (handleLogin(req, res)) return;
if (req.url === "/api/pages/breadcrumbs") {
// A root page: the CTE returns only the page itself.
sendJson(
res,
200,
breadcrumbsEnvelope([
{ id: ROOT_UUID, slugId: "rootSlug", title: "Infrastructure", spaceId: "sp1", parentPageId: null, hasChildren: false },
]),
);
return;
}
if (req.url === "/api/pages/sidebar-pages") {
sendJson(res, 200, {
success: true,
data: { items: [], meta: { hasNextPage: false, nextCursor: null } },
});
return;
}
sendJson(res, 404, {});
});
const client = new DocmostClient(baseURL, "user@example.com", "pw");
const result = await client.getPageContext(ROOT_UUID);
assert.deepEqual(result.page, {
pageId: ROOT_UUID,
title: "Infrastructure",
spaceId: "sp1",
});
assert.deepEqual(result.breadcrumbs, [], "root page: no ancestors");
assert.deepEqual(result.children, [], "no children");
});
// -----------------------------------------------------------------------------
// 3) A slugId input is resolved via /pages/info first (one extra request), then
// breadcrumbs/sidebar use the resolved UUID.
// -----------------------------------------------------------------------------
test("getPageContext: a slugId input is resolved via /pages/info", async () => {
let infoReqs = 0;
let infoBody = null;
let breadcrumbBody = null;
const { baseURL } = await spawn(async (req, res) => {
const raw = await readBody(req);
if (handleLogin(req, res)) return;
if (req.url === "/api/pages/info") {
infoReqs++;
infoBody = JSON.parse(raw || "{}");
// getPageRaw: slugId -> canonical UUID.
sendJson(res, 200, {
success: true,
data: { id: PAGE_UUID, slugId: "pageSlug", title: "Rack 12", spaceId: "sp1" },
});
return;
}
if (req.url === "/api/pages/breadcrumbs") {
breadcrumbBody = JSON.parse(raw || "{}");
sendJson(
res,
200,
breadcrumbsEnvelope([
{ id: ROOT_UUID, slugId: "rootSlug", title: "Infrastructure", spaceId: "sp1", parentPageId: null },
{ id: PAGE_UUID, slugId: "pageSlug", title: "Rack 12", spaceId: "sp1", parentPageId: ROOT_UUID },
]),
);
return;
}
if (req.url === "/api/pages/sidebar-pages") {
sendJson(res, 200, {
success: true,
data: { items: [], meta: { hasNextPage: false, nextCursor: null } },
});
return;
}
sendJson(res, 404, {});
});
const client = new DocmostClient(baseURL, "user@example.com", "pw");
const result = await client.getPageContext("pageSlug");
assert.equal(infoReqs, 1, "slugId resolved via one /pages/info");
assert.deepEqual(infoBody, { pageId: "pageSlug" }, "resolve posts the raw slugId");
assert.deepEqual(
breadcrumbBody,
{ pageId: PAGE_UUID },
"breadcrumbs posts the RESOLVED uuid, not the slugId",
);
assert.equal(result.page.pageId, PAGE_UUID, "page.pageId is the UUID");
assert.deepEqual(result.breadcrumbs, [
{ pageId: ROOT_UUID, title: "Infrastructure" },
]);
});
// -----------------------------------------------------------------------------
// 4) >20 children: cursor pagination returns ALL of them, no dupes (regression
// on the #442 bug class — getPageContext must not re-introduce a cap).
// -----------------------------------------------------------------------------
test("getPageContext: a page with >20 children returns ALL of them (no cap, no dupes)", async () => {
// 45 children spread over three cursor pages.
const all = Array.from({ length: 45 }, (_, i) => ({
id: `child-${i}`,
slugId: `slug-${i}`,
title: `Child ${i}`,
parentPageId: PAGE_UUID,
hasChildren: i % 2 === 0,
}));
const PAGES = {
"": { items: all.slice(0, 20), nextCursor: "c1" },
c1: { items: all.slice(20, 40), nextCursor: "c2" },
c2: { items: all.slice(40), nextCursor: null },
};
const { baseURL } = await spawn(async (req, res) => {
const raw = await readBody(req);
if (handleLogin(req, res)) return;
if (req.url === "/api/pages/breadcrumbs") {
sendJson(
res,
200,
breadcrumbsEnvelope([
{ id: PAGE_UUID, slugId: "pageSlug", title: "Big Parent", spaceId: "sp1", parentPageId: null },
]),
);
return;
}
if (req.url === "/api/pages/sidebar-pages") {
const body = JSON.parse(raw || "{}");
const page = PAGES[body.cursor ?? ""] ?? { items: [], nextCursor: null };
sendJson(res, 200, {
success: true,
data: {
items: page.items,
meta: { hasNextPage: page.nextCursor != null, nextCursor: page.nextCursor },
},
});
return;
}
sendJson(res, 404, {});
});
const client = new DocmostClient(baseURL, "user@example.com", "pw");
const result = await client.getPageContext(PAGE_UUID);
assert.equal(result.children.length, 45, "all 45 children returned");
const ids = result.children.map((c) => c.pageId);
assert.equal(new Set(ids).size, 45, "no duplicate children");
assert.deepEqual(ids, all.map((c) => c.id), "children in server order across cursor pages");
assert.equal(result.children[0].hasChildren, true, "hasChildren preserved (child 0)");
assert.equal(result.children[1].hasChildren, false, "hasChildren preserved (child 1)");
});
// -----------------------------------------------------------------------------
// 5) A nonexistent / inaccessible pageId -> a CLEAR error, NOT an empty object.
// -----------------------------------------------------------------------------
test("getPageContext: a bad/inaccessible pageId throws a clear error (not {})", async () => {
const { baseURL } = await spawn(async (req, res) => {
await readBody(req);
if (handleLogin(req, res)) return;
if (req.url === "/api/pages/breadcrumbs") {
// Server rejects an unknown/forbidden page.
sendJson(res, 404, { message: "Page not found" });
return;
}
sendJson(res, 404, {});
});
const client = new DocmostClient(baseURL, "user@example.com", "pw");
await assert.rejects(
() => client.getPageContext(PAGE_UUID),
(err) => {
assert.ok(err instanceof Error, "throws an Error");
return true;
},
"a 404 from breadcrumbs propagates as a thrown error, not a hollow {}",
);
});
// -----------------------------------------------------------------------------
// 6) An empty breadcrumbs chain (should never happen — the endpoint always
// includes the page itself) is treated as not-found, not a hollow {page:...}.
// -----------------------------------------------------------------------------
test("getPageContext: an empty breadcrumbs chain throws (defensive)", async () => {
const { baseURL } = await spawn(async (req, res) => {
await readBody(req);
if (handleLogin(req, res)) return;
if (req.url === "/api/pages/breadcrumbs") {
sendJson(res, 200, breadcrumbsEnvelope([]));
return;
}
sendJson(res, 404, {});
});
const client = new DocmostClient(baseURL, "user@example.com", "pw");
await assert.rejects(
() => client.getPageContext(PAGE_UUID),
/not found or inaccessible/,
"an empty chain is a clear error, not {}",
);
});
@@ -9,7 +9,6 @@ import {
__sessionCountForTests,
} from "../../build/lib/collab-session.js";
import { withPageLock } from "../../build/lib/page-lock.js";
import { DocmostClient } from "../../build/client.js";
// A stand-in for HocuspocusProvider: it shares the ydoc (so the real yjs
// read/transform/write in CollabSession.mutate runs unchanged), auto-completes
@@ -90,7 +89,6 @@ const ENV_KEYS = [
"MCP_COLLAB_SESSION_IDLE_MS",
"MCP_COLLAB_SESSION_MAX_AGE_MS",
"MCP_COLLAB_SESSION_MAX_ENTRIES",
"MCP_COLLAB_TOKEN_TTL_MS",
];
let savedEnv;
@@ -345,86 +343,6 @@ test("replaceImage-shaped flow: acquire under an EXTERNAL page lock does not dea
);
});
// --- #439: the collab-token cache is what makes the session cache ACTUALLY hit ---
//
// WHY these two tests exist (the #435 incident): the session registry keys on
// (wsUrl, pageId, token) for identity isolation, but BOTH production token
// sources mint a FRESH JWT on every call (the in-app provider re-signs a JWT
// whose iat/exp changes every second; the external MCP POSTs /auth/collab-token
// per call). The fresh token per call made the session-registry key unstable,
// so the prod hit-rate was 0% — connect storms, 25s timeouts, zombie sessions —
// while every other test in this file stayed green because they pass a FIXED
// "tok" string. The #439 fix is the per-client collab-token cache
// (DocmostClient.getCollabTokenWithReauth + MCP_COLLAB_TOKEN_TTL_MS); these
// tests drive the token through it with a source that returns a DIFFERENT
// fresh JWT per mint, exactly like prod, so a regression in EITHER the token
// cache or the registry keying turns them red.
//
// getCollabTokenWithReauth is TS-private, but the compiled JS exposes it; the
// tests call it directly because that is exactly the per-op composition of the
// production call sites (updatePage etc.: mint the token, then acquire).
test("#439 token cache ON: fresh-JWT-per-mint source, two ops => ONE connect (session cache hits)", async () => {
process.env.MCP_COLLAB_TOKEN_TTL_MS = "300000"; // cache ON (explicit, not default-dependent)
let mints = 0;
const client = new DocmostClient({
apiUrl: "http://h/api",
getToken: async () => "user-jwt",
// Like both prod sources: a DIFFERENT fresh JWT on every mint.
getCollabToken: async () => `fresh-jwt-${++mints}`,
});
// Op 1: mint the collab token through the client, then acquire + mutate.
const tok1 = await client.getCollabTokenWithReauth();
const s1 = await acquireCollabSession("page-1", tok1, "http://h/api");
await s1.mutate(() => docWith("one"));
// Op 2: the same identity mints again — the cache must serve the SAME token.
const tok2 = await client.getCollabTokenWithReauth();
const s2 = await acquireCollabSession("page-1", tok2, "http://h/api");
await s2.mutate(() => docWith("two"));
assert.equal(mints, 1, "the second op is served from the token cache");
assert.equal(tok2, tok1, "stable token => stable session-registry key");
assert.equal(s2, s1, "the live session is reused");
assert.equal(
FakeProvider.connectCount,
1,
"two mutations over one identity must cost exactly ONE real connect",
);
assert.equal(__sessionCountForTests(), 1);
});
test("#439 negative control: token cache OFF (TTL=0) reproduces the #435 churn — two ops => TWO connects", async () => {
process.env.MCP_COLLAB_TOKEN_TTL_MS = "0"; // explicit 0 disables the cache (fetch-per-call legacy)
let mints = 0;
const client = new DocmostClient({
apiUrl: "http://h/api",
getToken: async () => "user-jwt",
getCollabToken: async () => `fresh-jwt-${++mints}`,
});
const tok1 = await client.getCollabTokenWithReauth();
const s1 = await acquireCollabSession("page-1", tok1, "http://h/api");
await s1.mutate(() => docWith("one"));
const tok2 = await client.getCollabTokenWithReauth();
const s2 = await acquireCollabSession("page-1", tok2, "http://h/api");
await s2.mutate(() => docWith("two"));
assert.equal(mints, 2, "without the cache every op mints its own token");
assert.notEqual(tok2, tok1, "unstable token => unstable session-registry key");
assert.notEqual(s2, s1, "no session reuse");
assert.equal(
FakeProvider.connectCount,
2,
"a full reconnect per op — the #435 storm in miniature",
);
// The first session lingers under its now-unreachable key until its idle
// TTL — the zombie-session symptom of the incident.
assert.equal(__sessionCountForTests(), 2);
});
test("destroyAllSessions tears down every cached session", async () => {
await acquireCollabSession("page-1", "tok", "http://h/api");
await acquireCollabSession("page-2", "tok", "http://h/api");
@@ -1,86 +0,0 @@
// 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`);
});
@@ -1,229 +0,0 @@
// 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");
});
@@ -1,128 +0,0 @@
// Unit tests for the drawioEditCells operations (issue #425, acceptance #4):
// add / update / delete applied to the parsed model, with a cascade delete that
// removes container children AND every connected edge.
import { test } from "node:test";
import assert from "node:assert/strict";
import { applyCellOps, CellOpsError } from "../../build/lib/drawio-cell-ops.js";
import { parseCells } from "../../build/lib/drawio-xml.js";
const MODEL =
"<mxGraphModel><root><mxCell id=\"0\"/><mxCell id=\"1\" parent=\"0\"/>" +
'<mxCell id="grp" value="G" style="container=1;fillColor=none;" vertex="1" parent="1">' +
'<mxGeometry x="0" y="0" width="300" height="200" as="geometry"/></mxCell>' +
'<mxCell id="c1" value="Child1" style="rounded=1;" vertex="1" parent="grp">' +
'<mxGeometry x="10" y="10" width="80" height="40" as="geometry"/></mxCell>' +
'<mxCell id="c2" value="Child2" style="rounded=1;" vertex="1" parent="grp">' +
'<mxGeometry x="10" y="80" width="80" height="40" as="geometry"/></mxCell>' +
'<mxCell id="out" value="Outside" style="rounded=1;" vertex="1" parent="1">' +
'<mxGeometry x="400" y="10" width="80" height="40" as="geometry"/></mxCell>' +
'<mxCell id="e1" style="" edge="1" parent="1" source="c1" target="out">' +
'<mxGeometry relative="1" as="geometry"/></mxCell>' +
'<mxCell id="e2" style="" edge="1" parent="1" source="out" target="c2">' +
'<mxGeometry relative="1" as="geometry"/></mxCell>' +
"</root></mxGraphModel>";
const ids = (xml) =>
parseCells(xml)
.filter((c) => c.id !== "0" && c.id !== "1")
.map((c) => c.id)
.sort();
test("update changes ONLY the targeted cell", () => {
const out = applyCellOps(MODEL, [
{
op: "update",
cellId: "c1",
xml:
'<mxCell id="c1" value="Renamed" style="rounded=1;" vertex="1" parent="grp">' +
'<mxGeometry x="10" y="10" width="80" height="40" as="geometry"/></mxCell>',
},
]);
const cells = parseCells(out);
assert.equal(cells.find((c) => c.id === "c1").value, "Renamed");
// Every OTHER cell is untouched.
assert.equal(cells.find((c) => c.id === "c2").value, "Child2");
assert.equal(cells.find((c) => c.id === "out").value, "Outside");
assert.deepEqual(ids(out), ids(MODEL));
});
test("delete of a container removes its children AND the connected edges", () => {
const out = applyCellOps(MODEL, [{ op: "delete", cellId: "grp" }]);
// grp + c1 + c2 gone (cascade to children); e1 (c1->out) and e2 (out->c2)
// gone (cascade to connected edges); "out" survives.
assert.deepEqual(ids(out), ["out"]);
});
test("delete of a leaf only cascades to its connected edges, not siblings", () => {
const out = applyCellOps(MODEL, [{ op: "delete", cellId: "c1" }]);
// c1 gone + e1 (c1->out) gone; c2, out, grp, e2 survive.
assert.deepEqual(ids(out), ["c2", "e2", "grp", "out"]);
});
test("add appends a new cell", () => {
const out = applyCellOps(MODEL, [
{
op: "add",
xml:
'<mxCell id="new1" value="N" style="rounded=1;" vertex="1" parent="1">' +
'<mxGeometry x="500" y="10" width="80" height="40" as="geometry"/></mxCell>',
},
]);
assert.ok(parseCells(out).some((c) => c.id === "new1"));
});
test("delete never removes the sentinels", () => {
const out = applyCellOps(MODEL, [{ op: "delete", cellId: "out" }]);
const cells = parseCells(out);
assert.ok(cells.some((c) => c.id === "0"));
assert.ok(cells.some((c) => c.id === "1"));
});
test("errors: unknown update/delete target, duplicate add id, id mismatch", () => {
assert.throws(
() => applyCellOps(MODEL, [{ op: "update", cellId: "ghost", xml: '<mxCell id="ghost"/>' }]),
/does not exist/,
);
assert.throws(
() => applyCellOps(MODEL, [{ op: "delete", cellId: "ghost" }]),
/does not exist/,
);
assert.throws(
() => applyCellOps(MODEL, [{ op: "add", xml: '<mxCell id="c1"/>' }]),
/already exists/,
);
assert.throws(
() =>
applyCellOps(MODEL, [
{ op: "update", cellId: "c1", xml: '<mxCell id="c2"/>' },
]),
/ids are stable/,
);
assert.throws(() => applyCellOps(MODEL, []), CellOpsError);
});
test("an add op with two cells or a missing id is rejected", () => {
assert.throws(
() => applyCellOps(MODEL, [{ op: "add", xml: '<mxCell id="a"/><mxCell id="b"/>' }]),
/exactly one <mxCell>/,
);
assert.throws(
() => applyCellOps(MODEL, [{ op: "add", xml: '<mxCell value="x"/>' }]),
/missing an id/,
);
});
// --- SUGGESTION #5: sentinel cells are protected from delete ------------------
test("delete targeting a sentinel id is rejected (no wipe of the diagram body)", () => {
for (const sid of ["0", "1"]) {
assert.throws(
() => applyCellOps(MODEL, [{ op: "delete", cellId: sid }]),
(e) => e instanceof CellOpsError && /cannot delete sentinel cell/.test(e.message),
);
}
// A normal delete still works and the sentinels remain intact.
const out = applyCellOps(MODEL, [{ op: "delete", cellId: "out" }]);
const remaining = parseCells(out).map((c) => c.id);
assert.ok(remaining.includes("0") && remaining.includes("1"), "sentinels survive");
});
@@ -1,380 +0,0 @@
// Unit tests for the drawioFromGraph pipeline (issue #425, stage 3): the
// semantic graph -> ELK -> linter-clean XML assembler, plus the layout hints
// (pinned / sameLayerAs / layer) and incremental layout. Pure — no client, no
// network — so they run under `node --test` against the built lib.
import { test } from "node:test";
import assert from "node:assert/strict";
import {
buildFromGraph,
validateGraph,
resolveNodeStyle,
GraphValidationError,
} from "../../build/lib/drawio-graph.js";
import { getPreset } from "../../build/lib/drawio-presets.js";
import {
prepareModel,
parseCells,
computeQualityWarnings,
} from "../../build/lib/drawio-xml.js";
function cellsOf(xml) {
return parseCells(xml).filter((c) => c.id !== "0" && c.id !== "1");
}
function byId(xml) {
const m = new Map();
for (const c of parseCells(xml)) m.set(c.id, c);
return m;
}
// --- Acceptance #1: 15-node graph, 2 nested groups, AWS icons ---------------
test("from_graph: 15+ nodes, 2 nested groups, AWS icons -> 0 lint errors, 0 warnings, all icons resolve", async () => {
const awsIcons = [
"aws:lambda", "aws:dynamodb", "aws:api_gateway", "aws:s3", "aws:sqs",
"aws:sns", "aws:ec2", "aws:rds", "aws:cloudfront", "aws:elasticache",
"aws:kinesis", "aws:cognito", "aws:secrets_manager", "aws:cloudwatch",
];
const kinds = [
"service", "db", "gateway", "service", "queue", "queue", "service", "db",
"gateway", "db", "queue", "security", "security", "external",
];
const nodes = [];
for (let i = 0; i < 14; i++) {
nodes.push({
id: "n" + i,
label: "Node " + i,
kind: kinds[i],
icon: awsIcons[i],
group: i < 6 ? "sub1" : i < 10 ? "vpc1" : undefined,
});
}
nodes.push({ id: "ext1", label: "External Service", kind: "external" });
const graph = {
nodes,
groups: [
{ id: "vpc1", label: "VPC 10.0.0.0/16", kind: "vpc" },
{ id: "sub1", label: "Private Subnet", kind: "subnet", group: "vpc1" }, // NESTED
],
edges: [
{ from: "n0", to: "n1", kind: "sync" },
{ from: "n1", to: "n2", kind: "async" },
{ from: "n2", to: "n3" },
{ from: "n3", to: "n7", kind: "sync" },
{ from: "n7", to: "n8" },
{ from: "n8", to: "ext1", kind: "error" },
{ from: "n4", to: "n5" },
{ from: "n10", to: "n11" },
],
direction: "LR",
preset: "default",
};
const r = await buildFromGraph(graph, "full");
assert.equal(graph.nodes.length >= 15, true, "at least 15 nodes");
// All icons resolved — NO empty squares.
assert.equal(r.iconsMissing.length, 0, `unresolved icons: ${r.iconsMissing}`);
assert.equal(r.iconsResolved, 14);
// 0 lint errors + 0 quality-warnings.
const prepared = prepareModel(r.modelXml);
assert.equal(prepared.warnings.length, 0, prepared.warnings.join("\n"));
// Groups are TRANSPARENT containers.
const cells = byId(r.modelXml);
for (const gid of ["vpc1", "sub1"]) {
const g = cells.get(gid);
assert.ok(g, `${gid} present`);
assert.equal(g.styleMap.container, "1", `${gid} container=1`);
assert.equal(g.styleMap.fillColor, "none", `${gid} fillColor=none`);
assert.equal(g.styleMap.dropTarget, "1", `${gid} dropTarget=1`);
}
// The nested group sub1's parent IS vpc1 (nesting honoured).
assert.equal(cells.get("sub1").parent, "vpc1");
// A grouped node's parent is its group (relative coords).
assert.equal(cells.get("n0").parent, "sub1");
});
test("from_graph: an UNKNOWN icon degrades to a labelled generic shape (never empty)", async () => {
const graph = {
nodes: [
{ id: "a", label: "Mystery", kind: "service", icon: "not:a-real-icon-xyz" },
{ id: "b", label: "Plain", kind: "db" },
],
edges: [{ from: "a", to: "b" }],
};
const r = await buildFromGraph(graph, "full");
const cells = byId(r.modelXml);
// The node still carries its label and a real (non-empty) style with a fill.
assert.equal(cells.get("a").value, "Mystery");
assert.match(cells.get("a").style, /fillColor=/);
// It is reported as missing so the model can see the degradation.
assert.ok(r.iconsMissing.includes("a"));
});
// --- Acceptance #2: hints -----------------------------------------------------
test("from_graph: a pinned node stays at its exact coordinates", async () => {
const graph = {
nodes: [
{ id: "a", label: "A", pinned: { x: 40, y: 900 } },
{ id: "b", label: "B" },
{ id: "c", label: "C" },
],
edges: [{ from: "a", to: "b" }, { from: "b", to: "c" }],
};
const a = byId((await buildFromGraph(graph, "full")).modelXml).get("a");
assert.equal(a.geometry.x, 40);
assert.equal(a.geometry.y, 900);
});
test("from_graph: a sameLayerAs pair lands in the same layer (equal layer-axis coord)", async () => {
const graph = {
nodes: [
{ id: "x", label: "X" },
{ id: "y", label: "Y", sameLayerAs: "x" },
{ id: "z", label: "Z" },
],
edges: [{ from: "z", to: "x" }, { from: "z", to: "y" }],
direction: "LR", // layer axis = x
};
const cells = byId((await buildFromGraph(graph, "full")).modelXml);
assert.equal(cells.get("x").geometry.x, cells.get("y").geometry.x);
});
test("from_graph: an explicit layer index co-aligns nodes on the layer axis", async () => {
const graph = {
nodes: [
{ id: "p", label: "P", layer: 0 },
{ id: "q", label: "Q", layer: 0 },
{ id: "r", label: "R", layer: 1 },
],
edges: [{ from: "p", to: "r" }],
direction: "LR",
};
const cells = byId((await buildFromGraph(graph, "full")).modelXml);
assert.equal(cells.get("p").geometry.x, cells.get("q").geometry.x);
});
test("from_graph: TB direction snaps sameLayerAs on the Y axis", async () => {
const graph = {
nodes: [
{ id: "x", label: "X" },
{ id: "y", label: "Y", sameLayerAs: "x" },
{ id: "z", label: "Z" },
],
edges: [{ from: "z", to: "x" }, { from: "z", to: "y" }],
direction: "TB", // layer axis = y
};
const cells = byId((await buildFromGraph(graph, "full")).modelXml);
assert.equal(cells.get("x").geometry.y, cells.get("y").geometry.y);
});
// --- Acceptance #3: incremental never moves existing cells --------------------
test("from_graph incremental: adding a node does NOT move existing cells", async () => {
const existing = new Map([
["a", { x: 100, y: 100 }],
["b", { x: 400, y: 100 }],
]);
const graph = {
nodes: [
{ id: "a", label: "A" },
{ id: "b", label: "B" },
{ id: "cnew", label: "New" },
],
edges: [{ from: "a", to: "b" }, { from: "b", to: "cnew" }],
};
const cells = byId((await buildFromGraph(graph, "incremental", existing)).modelXml);
assert.deepEqual(
[cells.get("a").geometry.x, cells.get("a").geometry.y],
[100, 100],
);
assert.deepEqual(
[cells.get("b").geometry.x, cells.get("b").geometry.y],
[400, 100],
);
// The new node was placed and does not overlap the frozen block.
const cn = cells.get("cnew");
assert.ok(cn.geometry.y >= 200, "new node placed clear of the existing block");
});
// --- direction honoured -------------------------------------------------------
test("from_graph: LR vs RL flip the layout axis order", async () => {
const mk = (dir) => ({
nodes: [{ id: "s", label: "S" }, { id: "t", label: "T" }],
edges: [{ from: "s", to: "t" }],
direction: dir,
});
const lr = byId((await buildFromGraph(mk("LR"), "full")).modelXml);
// In LR the target sits to the RIGHT of the source.
assert.ok(lr.get("t").geometry.x > lr.get("s").geometry.x, "LR: t right of s");
const rl = byId((await buildFromGraph(mk("RL"), "full")).modelXml);
assert.ok(rl.get("t").geometry.x < rl.get("s").geometry.x, "RL: t left of s");
});
// --- validation ---------------------------------------------------------------
test("validateGraph: rejects duplicate node ids, unknown group/edge refs", () => {
assert.throws(
() => validateGraph({ nodes: [{ id: "a", label: "A" }, { id: "a", label: "B" }] }),
GraphValidationError,
);
assert.throws(
() => validateGraph({ nodes: [{ id: "a", label: "A", group: "ghost" }] }),
/unknown group/,
);
assert.throws(
() =>
validateGraph({
nodes: [{ id: "a", label: "A" }],
edges: [{ from: "a", to: "ghost" }],
}),
/resolves to no node/,
);
assert.throws(() => validateGraph({ nodes: [] }), /non-empty/);
});
test("resolveNodeStyle: kind maps to the preset palette slot for a generic node", () => {
const preset = getPreset("default");
const s = resolveNodeStyle(preset, { id: "d", label: "DB", kind: "db" });
assert.equal(s.iconResolved, false);
assert.match(s.style, /fillColor=#d5e8d4;strokeColor=#82b366/);
});
// --- the assembled XML is always linter-clean --------------------------------
test("from_graph: a plain cross-container-edge graph is linter-clean", async () => {
const graph = {
nodes: [
{ id: "a", label: "A", group: "g1" },
{ id: "b", label: "B", group: "g2" },
],
groups: [
{ id: "g1", label: "G1" },
{ id: "g2", label: "G2" },
],
edges: [{ from: "a", to: "b", label: "x" }],
};
const r = await buildFromGraph(graph, "full");
const prepared = prepareModel(r.modelXml); // throws on any lint error
assert.equal(prepared.warnings.length, 0, prepared.warnings.join("\n"));
// A cross-container edge is parented at the layer sentinel "1".
const edge = cellsOf(r.modelXml).find((c) => c.edge);
assert.equal(edge.parent, "1");
});
// --- CRITICAL #1: edge / group caps reject FAST (no layout, no OOM) -----------
test("validateGraph: an over-limit EDGE count is rejected before any layout", () => {
// 2 nodes, 200000 edges: passes node validation, would OOM graphToElk/runElk.
const edges = [];
for (let i = 0; i < 200_000; i++) edges.push({ from: "a", to: "b" });
const graph = { nodes: [{ id: "a", label: "A" }, { id: "b", label: "B" }], edges };
const t0 = Date.now();
assert.throws(
() => validateGraph(graph),
(e) => e instanceof GraphValidationError && /200000 edges .*max 1000/.test(e.message),
);
assert.ok(Date.now() - t0 < 1000, "must reject in well under a second (no layout)");
});
test("validateGraph: an over-limit GROUP count is rejected fast", () => {
const groups = [];
for (let i = 0; i < 600; i++) groups.push({ id: "g" + i, label: "G" });
const t0 = Date.now();
assert.throws(
() => validateGraph({ nodes: [{ id: "a", label: "A" }], groups }),
(e) => e instanceof GraphValidationError && /600 groups .*max 500/.test(e.message),
);
assert.ok(Date.now() - t0 < 1000);
});
test("validateGraph: exactly-at-cap edges/groups are accepted", () => {
const edges = [];
for (let i = 0; i < 1000; i++) edges.push({ from: "a", to: "b" });
assert.doesNotThrow(() =>
validateGraph({ nodes: [{ id: "a", label: "A" }, { id: "b", label: "B" }], edges }),
);
});
// --- WARNING #3: sameLayerAs spread -> 0 quality-warnings by construction -----
test("from_graph: a sameLayerAs chain of 5 yields 0 quality-warnings (cross-axis spread)", async () => {
const graph = {
nodes: [
{ id: "a", label: "A" },
{ id: "b", label: "B", sameLayerAs: "a" },
{ id: "c", label: "C", sameLayerAs: "b" },
{ id: "d", label: "D", sameLayerAs: "c" },
{ id: "e", label: "E", sameLayerAs: "d" },
],
edges: [{ from: "a", to: "b" }],
direction: "LR",
};
const r = await buildFromGraph(graph, "full");
const cells = parseCells(r.modelXml);
const warnings = computeQualityWarnings(cells);
assert.equal(warnings.length, 0, warnings.join("\n"));
// The four chained dependents share one layer-axis (x) coordinate...
const by = new Map(cells.map((c) => [c.id, c]));
const xs = ["b", "c", "d", "e"].map((id) => by.get(id).geometry.x);
assert.equal(new Set(xs).size, 1, "chained nodes must share the layer axis");
// ...but are spread on the cross axis (y) with distinct coordinates.
const ys = ["b", "c", "d", "e"].map((id) => by.get(id).geometry.y);
assert.equal(new Set(ys).size, 4, "chained nodes must not stack on the cross axis");
});
test("from_graph: pinned coords are honored verbatim; a negative pin is clamped non-negative", async () => {
const graph = {
nodes: [
{ id: "p1", label: "P1", pinned: { x: 100, y: 100 } },
// Two user-pinned nodes at (nearly) the same point: user intent, honored.
{ id: "p2", label: "P2", pinned: { x: -500, y: 100 } }, // out-of-bounds x -> clamped to 0
],
direction: "LR",
};
const r = await buildFromGraph(graph, "full");
const by = new Map(parseCells(r.modelXml).map((c) => [c.id, c]));
assert.equal(by.get("p1").geometry.x, 100);
assert.equal(by.get("p1").geometry.y, 100);
assert.equal(by.get("p2").geometry.x, 0, "negative pin x clamped to 0");
assert.equal(by.get("p2").geometry.y, 100, "pin y honored");
// A pinned overlap MAY warn — that's user-directed and documented; we only
// assert the coords are honored (the guarantee softening), not warning count.
});
// --- WARNING #4: incremental MERGE preserves unlisted existing cells ----------
test("from_graph incremental: an existing cell not in the new graph SURVIVES the add", async () => {
const existingModelXml =
'<mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/>' +
'<mxCell id="manual" value="Hand Placed" style="rounded=1;" vertex="1" parent="1">' +
'<mxGeometry x="900" y="900" width="120" height="60" as="geometry"/></mxCell>' +
'<mxCell id="old1" value="Old One" style="rounded=1;" vertex="1" parent="1">' +
'<mxGeometry x="40" y="40" width="120" height="60" as="geometry"/></mxCell>' +
"</root></mxGraphModel>";
const existingCoords = new Map([
["manual", { x: 900, y: 900 }],
["old1", { x: 40, y: 40 }],
]);
// The model sends ONLY the re-listed old1 + the new node — NOT "manual".
const graph = {
nodes: [
{ id: "old1", label: "Old One" },
{ id: "new1", label: "Added Node" },
],
edges: [{ from: "old1", to: "new1" }],
direction: "LR",
};
const r = await buildFromGraph(graph, "incremental", existingCoords, existingModelXml);
const by = new Map(parseCells(r.modelXml).map((c) => [c.id, c]));
// The unlisted hand-placed cell survives, verbatim coords.
assert.ok(by.has("manual"), "unlisted existing cell must not be dropped");
assert.equal(by.get("manual").geometry.x, 900);
assert.equal(by.get("manual").geometry.y, 900);
// The re-listed existing cell keeps its frozen coords; the new node is added.
assert.equal(by.get("old1").geometry.x, 40);
assert.equal(by.get("old1").geometry.y, 40);
assert.ok(by.has("new1"), "the newly added node is present");
});
@@ -1,114 +0,0 @@
// Unit tests for the Mermaid flowchart -> graph parser (issue #425, acceptance
// #6, OPTIONAL). Verifies a flowchart with a branch + a subgraph parses to a
// graph the from_graph pipeline renders as valid, editable drawio, and that a
// non-flowchart diagram is rejected with a clear error.
import { test } from "node:test";
import assert from "node:assert/strict";
import { mermaidToGraph, MermaidParseError } from "../../build/lib/drawio-mermaid.js";
import { buildFromGraph } from "../../build/lib/drawio-graph.js";
import { prepareModel } from "../../build/lib/drawio-xml.js";
test("flowchart with a branch + subgraph -> a valid, editable drawio", async () => {
const mm = `flowchart LR
A[Start] --> B{Decision}
B -->|yes| C[(Database)]
B -->|no| D[End]
subgraph backend [Backend Services]
C
E([Cache])
end
D -.-> E`;
const graph = mermaidToGraph(mm);
// Direction + nodes + a group + labelled/dashed edges parsed.
assert.equal(graph.direction, "LR");
const nodeIds = graph.nodes.map((n) => n.id).sort();
assert.deepEqual(nodeIds, ["A", "B", "C", "D", "E"]);
// The decision `{}` maps to the queue palette; the `[(db)]` to db.
assert.equal(graph.nodes.find((n) => n.id === "B").kind, "queue");
assert.equal(graph.nodes.find((n) => n.id === "C").kind, "db");
// The subgraph became a group and claimed its members.
assert.equal(graph.groups.length, 1);
assert.equal(graph.groups[0].id, "backend");
assert.equal(graph.nodes.find((n) => n.id === "C").group, "backend");
assert.equal(graph.nodes.find((n) => n.id === "E").group, "backend");
// A pipe label and a dotted (async) edge.
const yes = graph.edges.find((e) => e.from === "B" && e.to === "C");
assert.equal(yes.label, "yes");
const dotted = graph.edges.find((e) => e.from === "D" && e.to === "E");
assert.equal(dotted.kind, "async");
// The whole thing renders linter-clean.
const built = await buildFromGraph(graph, "full");
const prepared = prepareModel(built.modelXml);
assert.equal(prepared.warnings.length, 0, prepared.warnings.join("\n"));
});
test("graph TD header sets a top-down direction", () => {
const g = mermaidToGraph("graph TD\n X --> Y");
assert.equal(g.direction, "TB");
assert.deepEqual(g.nodes.map((n) => n.id).sort(), ["X", "Y"]);
});
test("a chained connection A --> B --> C yields two edges", () => {
const g = mermaidToGraph("flowchart LR\n A[a] --> B[b] --> C[c]");
const pairs = g.edges.map((e) => `${e.from}->${e.to}`).sort();
assert.deepEqual(pairs, ["A->B", "B->C"]);
});
test("a non-flowchart diagram is rejected with a clear error", () => {
assert.throws(
() => mermaidToGraph("sequenceDiagram\n Alice->>Bob: Hi"),
/only 'flowchart'\/'graph' is supported/,
);
assert.throws(() => mermaidToGraph(""), MermaidParseError);
});
// --- CRITICAL #2 / NIT: input-size bounds reject FAST (no OOM) ----------------
test("mermaidToGraph: an over-length input is rejected before parsing (fast)", () => {
const huge = "flowchart LR\n" + "A-->B\n".repeat(60_000); // ~360 KB > 200 KB cap
const t0 = Date.now();
assert.throws(
() => mermaidToGraph(huge),
(e) => e instanceof MermaidParseError && /max 200000/.test(e.message),
);
assert.ok(Date.now() - t0 < 1000, "must reject in well under a second (no parse)");
});
test("mermaidToGraph: an over-line-count input is rejected fast", () => {
const many = "flowchart LR\n" + "A\n".repeat(25_000); // > 20000 line cap
const t0 = Date.now();
assert.throws(
() => mermaidToGraph(many),
(e) => e instanceof MermaidParseError && /max 20000/.test(e.message),
);
assert.ok(Date.now() - t0 < 1000);
});
test("mermaidToGraph: too many subgraphs is rejected", () => {
let src = "flowchart LR\n";
for (let i = 0; i < 600; i++) src += `subgraph s${i}\nend\n`;
assert.throws(
() => mermaidToGraph(src),
(e) => e instanceof MermaidParseError && /too many subgraphs .*max 500/.test(e.message),
);
});
test("mermaidToGraph: an over-long connection chain throws (NON-silent truncation)", () => {
const chain =
"flowchart LR\n" +
Array.from({ length: 600 }, (_, i) => "N" + i).join("-->");
assert.throws(
() => mermaidToGraph(chain),
(e) => e instanceof MermaidParseError && /chain exceeds 500 nodes/.test(e.message),
);
});
test("mermaidToGraph: a chain of 60 nodes parses (no silent 50-node truncation)", () => {
const chain =
"flowchart LR\n" +
Array.from({ length: 60 }, (_, i) => "N" + i).join("-->");
const g = mermaidToGraph(chain);
assert.equal(g.nodes.length, 60, "all 60 chained nodes are kept");
});
@@ -1,117 +0,0 @@
// Snapshot + invariant tests for the semantic presets (issue #425, acceptance
// #5): every node kind has a style-string per preset, and colorblind-safe uses
// only the Okabe-Ito palette (no problematic color pairs).
import { test } from "node:test";
import assert from "node:assert/strict";
import {
getPreset,
genericNodeStyle,
edgeStyle,
groupStyle,
NODE_KINDS,
EDGE_KINDS,
PRESET_NAMES,
} from "../../build/lib/drawio-presets.js";
// The Okabe-Ito qualitative palette (8 colours distinguishable under the common
// colour-vision deficiencies). colorblind-safe MUST draw its strokes from here.
const OKABE_ITO = [
"#000000", "#e69f00", "#56b4e9", "#009e73",
"#f0e442", "#0072b2", "#d55e00", "#cc79a7",
].map((s) => s.toLowerCase());
// The exact base-palette fills from the issue's table (a snapshot: a change to
// the default palette is a deliberate, reviewed edit — this catches accidents).
const DEFAULT_FILLS = {
service: "#dae8fc",
db: "#d5e8d4",
queue: "#fff2cc",
gateway: "#ffe6cc",
error: "#f8cecc",
external: "#f5f5f5",
security: "#e1d5e7",
};
const DEFAULT_STROKES = {
service: "#6c8ebf",
db: "#82b366",
queue: "#d6b656",
gateway: "#d79b00",
error: "#b85450",
external: "#666666",
security: "#9673a6",
};
test("every node kind has a style-string in every preset", () => {
for (const p of PRESET_NAMES) {
const preset = getPreset(p);
for (const kind of NODE_KINDS) {
const style = genericNodeStyle(preset, kind);
assert.match(style, /fillColor=#[0-9a-fA-F]{6}/, `${p}/${kind} has a fill`);
assert.match(style, /strokeColor=#[0-9a-fA-F]{6}/, `${p}/${kind} has a stroke`);
assert.match(style, /fontColor=#[0-9a-fA-F]{6}/, `${p}/${kind} has a font`);
}
}
});
test("default preset matches the issue's palette snapshot", () => {
const preset = getPreset("default");
for (const kind of NODE_KINDS) {
const style = genericNodeStyle(preset, kind);
assert.ok(
style.includes(`fillColor=${DEFAULT_FILLS[kind]}`),
`default/${kind} fill ${DEFAULT_FILLS[kind]}`,
);
assert.ok(
style.includes(`strokeColor=${DEFAULT_STROKES[kind]}`),
`default/${kind} stroke ${DEFAULT_STROKES[kind]}`,
);
}
});
test("colorblind-safe strokes come ONLY from the Okabe-Ito palette", () => {
const preset = getPreset("colorblind-safe");
const usedStrokes = new Set();
for (const kind of NODE_KINDS) {
const stroke = preset.nodes[kind].strokeColor.toLowerCase();
assert.ok(
OKABE_ITO.includes(stroke),
`colorblind-safe/${kind} stroke ${stroke} is NOT Okabe-Ito`,
);
usedStrokes.add(stroke);
}
// No two kinds share a stroke (each is a distinct, distinguishable hue) — the
// "no problematic color pairs" acceptance: distinct Okabe-Ito hues.
assert.equal(
usedStrokes.size,
NODE_KINDS.length,
"each kind gets a distinct Okabe-Ito stroke",
);
});
test("edge kinds: sync solid, async dashed, error red-dashed", () => {
const preset = getPreset("default");
const sync = edgeStyle(preset, "sync");
const async_ = edgeStyle(preset, "async");
const error = edgeStyle(preset, "error");
assert.doesNotMatch(sync, /dashed=1/, "sync is solid");
assert.match(async_, /dashed=1/, "async is dashed");
assert.match(error, /dashed=1/, "error is dashed");
assert.match(error, /strokeColor=#DD344C/i, "error is red");
// An unknown edge kind falls back to sync (solid).
assert.doesNotMatch(edgeStyle(preset, "weird"), /dashed=1/);
});
test("group style is always transparent (fillColor=none;container=1;dropTarget=1)", () => {
for (const p of PRESET_NAMES) {
const style = groupStyle(getPreset(p));
assert.match(style, /fillColor=none/, `${p} group transparent`);
assert.match(style, /container=1/, `${p} group is a container`);
assert.match(style, /dropTarget=1/, `${p} group is a drop target`);
}
});
test("EDGE_KINDS / NODE_KINDS constants match the palette", () => {
const preset = getPreset("default");
for (const k of NODE_KINDS) assert.ok(preset.nodes[k], `node kind ${k}`);
for (const k of EDGE_KINDS) assert.ok(preset.edges[k], `edge kind ${k}`);
});
@@ -1,85 +0,0 @@
// Drift guards for the stage-3 drawio tools (issue #425): the three high-level
// tools must be in the shared registry, routed in SERVER_INSTRUCTIONS, expose
// the right schema fields, and carry an `execute` (they call CLIENT methods, so
// unlike drawioShapes/drawioGuide they are NOT inlineBothHosts).
import { test } from "node:test";
import assert from "node:assert/strict";
import { SERVER_INSTRUCTIONS } from "../../build/index.js";
import { SHARED_TOOL_SPECS } from "../../build/tool-specs.js";
const NEW = ["drawioEditCells", "drawioFromGraph", "drawioFromMermaid"];
test("the three stage-3 tools are in the shared registry (deferred, camelCase)", () => {
for (const name of NEW) {
const spec = SHARED_TOOL_SPECS[name];
assert.ok(spec, `${name} missing from registry`);
assert.equal(spec.mcpName, name);
assert.equal(spec.inAppKey, name);
assert.equal(spec.tier, "deferred");
}
});
test("the stage-3 tools carry an execute (client-backed, NOT inlineBothHosts)", () => {
for (const name of NEW) {
const spec = SHARED_TOOL_SPECS[name];
assert.equal(typeof spec.execute, "function", `${name} needs an execute`);
assert.notEqual(spec.inlineBothHosts, true, `${name} must not be inlineBothHosts`);
}
});
test("the stage-3 tools are routed in SERVER_INSTRUCTIONS", () => {
for (const name of NEW) {
assert.match(SERVER_INSTRUCTIONS, new RegExp(`\\b${name}\\b`), `${name} missing from guide`);
}
});
test("drawioFromGraph exposes graph + direction/preset/layout params", () => {
const shape = SHARED_TOOL_SPECS.drawioFromGraph.buildShape(makeZodStub());
for (const key of ["pageId", "graph", "position", "direction", "preset", "layout", "node"]) {
assert.ok(key in shape, `drawioFromGraph missing ${key}`);
}
});
test("drawioEditCells exposes operations + baseHash", () => {
const shape = SHARED_TOOL_SPECS.drawioEditCells.buildShape(makeZodStub());
for (const key of ["pageId", "node", "operations", "baseHash"]) {
assert.ok(key in shape, `drawioEditCells missing ${key}`);
}
});
test("drawioFromMermaid exposes a mermaid param", () => {
const shape = SHARED_TOOL_SPECS.drawioFromMermaid.buildShape(makeZodStub());
assert.ok("mermaid" in shape);
});
test("the hard-rules block is injected into edit_cells (raw <mxCell> ops) but NOT from_graph/from_mermaid", () => {
// edit_cells takes raw <mxCell> xml in add/update ops, so it surfaces the XML
// rules. from_graph/from_mermaid NEVER expose XML to the model (the whole point
// is the model never writes a style/coord), so the hard rules would be noise.
assert.match(SHARED_TOOL_SPECS.drawioEditCells.description, /sentinels are MANDATORY/);
assert.doesNotMatch(SHARED_TOOL_SPECS.drawioFromGraph.description, /sentinels are MANDATORY/);
assert.doesNotMatch(SHARED_TOOL_SPECS.drawioFromMermaid.description, /sentinels are MANDATORY/);
});
test("routing prose distinguishes from_graph (architectures) vs from_mermaid (standard)", () => {
// The EDIT-section routing sentence must mention the semantic tools' intents.
assert.match(SERVER_INSTRUCTIONS, /drawioFromGraph[\s\S]*architecture|architecture[\s\S]*drawioFromGraph/i);
assert.match(SERVER_INSTRUCTIONS, /drawioFromMermaid[\s\S]*flowchart|flowchart[\s\S]*drawioFromMermaid/i);
});
// Tiny zod stub (buildShape only calls string/number/enum/array/object +
// chained min/optional/describe — all return `this`; object() returns a chain
// too so nested schemas resolve).
function makeZodStub() {
const chain = new Proxy(
{},
{ get: (_t, p) => (p === "parse" ? () => ({}) : () => chain) },
);
return {
string: () => chain,
number: () => chain,
enum: () => chain,
array: () => chain,
object: () => chain,
};
}
+1 -93
View File
@@ -1,11 +1,7 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import {
filterComment,
filterPage,
filterSearchResult,
} from "../../build/lib/filters.js";
import { filterComment, filterPage } from "../../build/lib/filters.js";
test("filterComment includes resolvedAt/resolvedById as null when absent", () => {
const result = filterComment({
@@ -175,91 +171,3 @@ test("filterPage includes both content and subpages together", () => {
assert.equal(result.content, "body");
assert.deepEqual(result.subpages, [{ id: "s1", title: "Sub" }]);
});
// --- filterSearchResult (#443 agent-lookup contract) -------------------------
test("filterSearchResult maps the lookup shape to {pageId,title,path,snippet,score}", () => {
const result = filterSearchResult({
id: "0199aa-uuid",
slugId: "slug-secret",
title: "backup-srv.local",
parentPageId: "0199pp",
path: ["Infrastructure", "Datacenter A", "Servers"],
snippet: "…IP: 10.0.12.5. Debian 12…",
score: 0.92,
});
assert.deepEqual(result, {
pageId: "0199aa-uuid",
title: "backup-srv.local",
path: ["Infrastructure", "Datacenter A", "Servers"],
snippet: "…IP: 10.0.12.5. Debian 12…",
score: 0.92,
});
});
test("filterSearchResult NEVER exposes slugId (pageId is the only identifier)", () => {
const result = filterSearchResult({
id: "uuid-1",
slugId: "slug-1",
title: "t",
path: [],
snippet: "s",
score: 0.1,
});
assert.equal("slugId" in result, false);
assert.equal("id" in result, false);
assert.equal(result.pageId, "uuid-1");
});
test("filterSearchResult root page yields path: []", () => {
const result = filterSearchResult({
id: "uuid-root",
title: "Root",
path: [],
snippet: "s",
score: 0.5,
});
assert.deepEqual(result.path, []);
});
test("filterSearchResult degrades a legacy FTS hit (no lookup fields)", () => {
// Stock upstream stripped the opt-in DTO fields → legacy shape with
// highlight + rank and no path/snippet/score.
const result = filterSearchResult({
id: "uuid-legacy",
slugId: "slug-legacy",
title: "Legacy",
parentPageId: null,
rank: 0.37,
highlight: "…matched <b>text</b>…",
space: { id: "sp1", name: "Space" },
});
assert.equal(result.pageId, "uuid-legacy");
assert.equal(result.title, "Legacy");
// snippet falls back to highlight, score to rank, path to [].
assert.equal(result.snippet, "…matched <b>text</b>…");
assert.equal(result.score, 0.37);
assert.deepEqual(result.path, []);
assert.equal("slugId" in result, false);
});
test("filterSearchResult is null-safe on missing snippet/score/path", () => {
const result = filterSearchResult({ id: "u", title: "t" });
assert.equal(result.pageId, "u");
assert.equal(result.snippet, "");
assert.equal(result.score, 0);
assert.deepEqual(result.path, []);
});
test("filterSearchResult ignores a non-array path", () => {
const result = filterSearchResult({
id: "u",
title: "t",
path: "not-an-array",
snippet: "s",
score: 1,
});
assert.deepEqual(result.path, []);
});
@@ -165,70 +165,6 @@ test("insertNode spec exists, describes markdown+node XOR, builds the full ancho
);
});
// #443: getTree — a space's page hierarchy (or a subtree) in one request.
test("getTree spec exists on both hosts, builds { spaceId, rootPageId?, maxDepth? }", () => {
const spec = SHARED_TOOL_SPECS.getTree;
assert.ok(spec, "getTree spec missing");
assert.equal(spec.mcpName, "getTree");
assert.equal(spec.inAppKey, "getTree");
// Shared spec: registered on BOTH hosts.
assert.notEqual(spec.inAppOnly, true);
assert.notEqual(spec.mcpOnly, true);
const shape = spec.buildShape(z);
assert.deepEqual(Object.keys(shape).sort(), ["maxDepth", "rootPageId", "spaceId"]);
const schema = z.object(shape);
// spaceId required; rootPageId + maxDepth optional.
assert.doesNotThrow(() => schema.parse({ spaceId: "sp1" }));
assert.throws(() => schema.parse({}));
assert.doesNotThrow(() =>
schema.parse({ spaceId: "sp1", rootPageId: "p1", maxDepth: 2 }),
);
// maxDepth is an integer >= 1.
assert.throws(() => schema.parse({ spaceId: "sp1", maxDepth: 0 }));
assert.throws(() => schema.parse({ spaceId: "sp1", maxDepth: 1.5 }));
// The description advertises the output node shape, rootPageId, maxDepth, and
// steers away from the deprecated listPages tree:true.
assert.match(spec.description, /pageId/);
assert.match(spec.description, /rootPageId/);
assert.match(spec.description, /maxDepth/);
assert.match(spec.description, /hasChildren/);
assert.match(spec.description, /listPages tree:true/);
});
// #443: getPageContext — a page's breadcrumbs + direct children in one call.
test("getPageContext spec exists on both hosts, builds { pageId }", () => {
const spec = SHARED_TOOL_SPECS.getPageContext;
assert.ok(spec, "getPageContext spec missing");
assert.equal(spec.mcpName, "getPageContext");
assert.equal(spec.inAppKey, "getPageContext");
// Shared spec: registered on BOTH hosts.
assert.notEqual(spec.inAppOnly, true);
assert.notEqual(spec.mcpOnly, true);
const shape = spec.buildShape(z);
assert.deepEqual(Object.keys(shape).sort(), ["pageId"]);
const schema = z.object(shape);
// pageId required.
assert.doesNotThrow(() => schema.parse({ pageId: "p1" }));
assert.throws(() => schema.parse({}));
// The description advertises the output shape (page/breadcrumbs/children) and
// the root-page empty-breadcrumbs contract.
assert.match(spec.description, /breadcrumbs/);
assert.match(spec.description, /children/);
assert.match(spec.description, /hasChildren/);
assert.match(spec.description, /getTree/);
});
// #443: listPages tree:true is deprecated in favour of getTree.
test("listPages description deprecates tree:true and points at getTree", () => {
const spec = SHARED_TOOL_SPECS.listPages;
assert.match(spec.description, /DEPRECATED/i);
assert.match(spec.description, /getTree/);
});
test("no-arg specs (getWorkspace/listSpaces/listShares) omit buildShape", () => {
for (const key of ["getWorkspace", "listSpaces", "listShares"]) {
assert.equal(SHARED_TOOL_SPECS[key].buildShape, undefined, `${key} should be no-arg`);
-161
View File
@@ -137,164 +137,3 @@ test("buildPageTree output shape is lean (drops position/parentPageId/hasChildre
assert.equal("hasChildren" in node, false);
assert.equal("spaceId" in node, false);
});
// ---------------------------------------------------------------------------
// #443 getTree output shape: { pageId, title, children?, hasChildren? }
// ---------------------------------------------------------------------------
// A small representative space used across the getTree tests:
// r1 (Infrastructure)
// c1 (Datacenter A)
// g1 (Servers) [leaf]
// c2 (Datacenter B) [leaf]
// r2 (Notes) [leaf]
const SAMPLE = [
{ id: "r2", slugId: "s-r2", title: "Notes", position: "a1", icon: "📝", hasChildren: false },
{ id: "r1", slugId: "s-r1", title: "Infrastructure", position: "a0", icon: "🏢", hasChildren: true },
{ id: "c2", slugId: "s-c2", title: "Datacenter B", position: "b1", parentPageId: "r1", icon: "🅱️", hasChildren: false },
{ id: "c1", slugId: "s-c1", title: "Datacenter A", position: "b0", parentPageId: "r1", icon: "🅰️", hasChildren: true },
{ id: "g1", slugId: "s-g1", title: "Servers", position: "c0", parentPageId: "c1", icon: "🖥️", hasChildren: false },
];
test("getTree shape: correct nesting + order-by-position, only {pageId,title,children?}, no leak", () => {
const tree = buildPageTree(SAMPLE, { shape: "getTree" });
// Roots sorted by position: r1 (a0) before r2 (a1).
assert.deepEqual(
tree.map((n) => n.pageId),
["r1", "r2"],
);
// r1's children sorted by position: c1 (b0) before c2 (b1).
assert.deepEqual(
tree[0].children.map((n) => n.pageId),
["c1", "c2"],
);
// Deep nesting: g1 under c1.
assert.deepEqual(
tree[0].children[0].children.map((n) => n.pageId),
["g1"],
);
// No slugId/icon/position/parentPageId/hasChildren leak on any node.
const walk = (nodes) => {
for (const n of nodes) {
assert.deepEqual(
Object.keys(n).sort(),
n.children ? ["children", "pageId", "title"] : ["pageId", "title"],
`unexpected keys on ${n.pageId}: ${Object.keys(n)}`,
);
assert.equal("slugId" in n, false);
assert.equal("icon" in n, false);
assert.equal("position" in n, false);
assert.equal("parentPageId" in n, false);
// Fully-expanded tree (no maxDepth): hasChildren never set.
assert.equal("hasChildren" in n, false);
if (n.children) walk(n.children);
}
};
walk(tree);
});
test("getTree maxDepth:1 returns roots only, each with hasChildren from the flat item", () => {
const tree = buildPageTree(SAMPLE, { shape: "getTree", maxDepth: 1 });
assert.deepEqual(
tree.map((n) => n.pageId),
["r1", "r2"],
);
// No children arrays at depth 1 when maxDepth:1.
for (const n of tree) assert.equal("children" in n, false);
// r1 has children on the server -> hasChildren:true; r2 is a leaf -> omitted.
assert.equal(tree[0].hasChildren, true);
assert.equal("hasChildren" in tree[1], false);
});
test("getTree maxDepth:2 cuts grandchildren; hasChildren only on the cut interior node", () => {
const tree = buildPageTree(SAMPLE, { shape: "getTree", maxDepth: 2 });
const r1 = tree[0];
// Depth-1 node r1 was EXPANDED (its children are present) -> no hasChildren.
assert.equal("hasChildren" in r1, false);
assert.equal(r1.children.length, 2);
const [c1, c2] = r1.children;
// c1 is at depth 2 (the cut) and has children on the server -> hasChildren:true,
// and its grandchild g1 is NOT present.
assert.equal(c1.pageId, "c1");
assert.equal("children" in c1, false);
assert.equal(c1.hasChildren, true);
// c2 is at depth 2 but is a leaf on the server -> hasChildren omitted.
assert.equal(c2.pageId, "c2");
assert.equal("children" in c2, false);
assert.equal("hasChildren" in c2, false);
// r2 is a depth-1 leaf -> no hasChildren, no children.
assert.equal("hasChildren" in tree[1], false);
});
test("getTree hasChildren is set ONLY on depth-cut nodes (not leaves, not expanded interior nodes)", () => {
// Full tree (no cut): NO node anywhere carries hasChildren.
const full = buildPageTree(SAMPLE, { shape: "getTree" });
const anyHasChildren = (nodes) =>
nodes.some((n) => "hasChildren" in n || (n.children && anyHasChildren(n.children)));
assert.equal(anyHasChildren(full), false);
});
test("getTree orphan (parent filtered out) surfaces as a root, not dropped", () => {
const tree = buildPageTree(
[
{ id: "root", slugId: "s-root", title: "Root", position: "a0", hasChildren: true },
// parentPageId points at an id NOT in the flat list (parent filtered by perms).
{ id: "orphan", slugId: "s-orphan", title: "Orphan", position: "a1", parentPageId: "gone", hasChildren: false },
],
{ shape: "getTree" },
);
assert.deepEqual(
tree.map((n) => n.pageId).sort(),
["orphan", "root"],
);
const orphan = tree.find((n) => n.pageId === "orphan");
assert.equal("children" in orphan, false);
assert.equal("hasChildren" in orphan, false);
});
test("getTree rootPageId path: a seeded single-root subtree keeps the getTree shape", () => {
// Simulate the server seeding the CTE with the subtree root c1: the flat list
// it returns contains c1 (now a root, parent absent) + its descendant g1.
const subtree = [
{ id: "c1", slugId: "s-c1", title: "Datacenter A", position: "b0", hasChildren: true },
{ id: "g1", slugId: "s-g1", title: "Servers", position: "c0", parentPageId: "c1", hasChildren: false },
];
const tree = buildPageTree(subtree, { shape: "getTree" });
assert.equal(tree.length, 1);
assert.equal(tree[0].pageId, "c1");
assert.deepEqual(
tree[0].children.map((n) => n.pageId),
["g1"],
);
assert.equal("slugId" in tree[0], false);
});
test("getTree maxDepth<=0 / non-finite is treated as no cut (whole tree)", () => {
for (const bad of [0, -3, NaN, Infinity, undefined]) {
const tree = buildPageTree(SAMPLE, { shape: "getTree", maxDepth: bad });
// Grandchild g1 present -> no cut applied.
assert.deepEqual(
tree[0].children[0].children.map((n) => n.pageId),
["g1"],
`maxDepth=${bad} should not cut`,
);
}
});
test("buildPageTree() with no options is byte-identical to the historic lean call", () => {
// Guard the existing callers: buildPageTree(pages) must be unchanged by the
// additive options param.
const withoutOpts = buildPageTree(SAMPLE);
const withEmptyOpts = buildPageTree(SAMPLE, {});
assert.deepEqual(withoutOpts, withEmptyOpts);
// And it is the lean {id,slugId,title,children?} shape, not the getTree shape.
assert.deepEqual(Object.keys(withoutOpts[0]).sort(), ["children", "id", "slugId", "title"]);
});
-79
View File
@@ -1,79 +0,0 @@
#!/usr/bin/env bash
# Boot-smoke for the exact Docker image that is about to be pushed (issue #476).
#
# Retrospective class "local logic is right, the integration property was never
# checked" (#353/#452/#361): every other CI job builds and tests code from the
# working tree, but the IMAGE watchtower pulls was never actually started
# anywhere before this gate. This script boots the built image against the
# publish job's postgres/redis services and asserts four integration
# properties end-to-end:
# S1 the app boots and /api/health answers (startup migrator + boot)
# S2 the first-run workspace setup endpoint works (API + DB writes)
# S3 the client dist is inside the image and served
# S4 hashed assets are served immutable (#452) with the precompressed
# brotli copy shipped in the image
set -euo pipefail
IMAGE="${1:?usage: image-smoke.sh <image>}"
fail() { echo "FAIL: $*"; exit 1; }
# Boot the exact image that will be pushed, wired to the job services via host
# network (postgres on localhost:5432, redis on localhost:6379). The container
# is deliberately NOT removed on failure so the workflow's dump-on-failure step
# can read `docker logs gitmost-smoke`.
docker run -d --name gitmost-smoke --network host \
-e DATABASE_URL=postgresql://docmost:docmost@localhost:5432/docmost \
-e REDIS_URL=redis://localhost:6379 \
-e APP_SECRET=ci-smoke-secret-change-me-min-32-characters \
-e APP_URL=http://localhost:3000 \
"$IMAGE"
# S1: wait for /api/health — covers the startup migrator + boot inside the
# shipped image (#361-boot, #353 runtime class): a migration the Kysely startup
# migrator rejects, or a runtime module missing from the image, dies right here.
healthy=0
for _ in $(seq 1 60); do
if curl -fsS http://localhost:3000/api/health > /dev/null 2>&1; then
healthy=1
break
fi
sleep 2
done
[ "$healthy" -eq 1 ] || fail "S1: /api/health did not answer within 120s (boot or startup migration failed)"
echo "OK S1: image booted and /api/health answers"
# S2: the first-run workspace setup works end-to-end (controller -> service ->
# DB write chain inside the shipped image, not just a static health probe).
curl -fsS -X POST http://localhost:3000/api/auth/setup \
-H "Content-Type: application/json" \
-d '{"name":"Smoke","email":"smoke@example.com","password":"SmokePassword123","workspaceName":"Smoke"}' \
> /dev/null || fail "S2: POST /api/auth/setup failed"
echo "OK S2: workspace setup succeeded"
# S3: the client dist is actually inside the image and served — the SPA HTML
# must reference hashed /assets/ bundles (a broken client COPY in the
# Dockerfile would serve an empty shell that every other job stays green on).
HTML=$(curl -fsS http://localhost:3000/) || fail "S3: fetching / failed"
grep -q '/assets/' <<<"$HTML" || fail "S3: served HTML references no /assets/ bundle (client dist missing from the image?)"
echo "OK S3: client dist served (HTML references /assets/)"
# S4: hashed /assets/ files must be served with an immutable cache-control
# (#452 class: static.module.ts resolveStaticAssetHeaders owns the header) AND
# with the precompressed brotli neighbour. Both checks are mandatory — verified
# against the code: resolveStaticAssetHeaders marks every /assets/ path
# immutable, and the client build (vite-plugin-compression2, include covers
# .js) emits a .br copy next to every bundle that the Dockerfile ships and
# @fastify/static serves via preCompressed:true.
ASSET=$(grep -oE '/assets/[A-Za-z0-9._@/-]+\.js' <<<"$HTML" | head -1 || true)
[ -n "$ASSET" ] || fail "S4: no /assets/*.js path found in the served HTML"
HDRS=$(curl -fsSI -H 'Accept-Encoding: br' "http://localhost:3000$ASSET") || fail "S4: HEAD $ASSET failed"
grep -qi '^cache-control:.*immutable' <<<"$HDRS" || fail "S4: $ASSET served without an immutable cache-control (#452)"
echo "OK S4: hashed asset served with immutable cache-control"
grep -qi '^content-encoding:.*br' <<<"$HDRS" || fail "S4: $ASSET not served brotli-precompressed (content-encoding: br missing)"
echo "OK S4: hashed asset served with the precompressed brotli copy"
# Remove the container ONLY on success, so the failure path keeps it around for
# the workflow's "Dump smoke container log on failure" step.
docker rm -f gitmost-smoke > /dev/null
echo "OK image smoke passed"