Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 agent_coder
|
0f5f048ca2 |
@@ -1,13 +1,5 @@
|
||||
name: Test
|
||||
|
||||
# NO `paths:` filter on purpose (issue #447). The tool-spec REGISTRY is split
|
||||
# across two packages that MUST stay in sync: the specs live in `packages/mcp`
|
||||
# but the parity/tier guard tests that read them live in the `apps/server` jest
|
||||
# suite. A PR touching only `packages/mcp/**` must therefore still run the SERVER
|
||||
# suite (and vice-versa), or an in-app wiring break slips through green and only
|
||||
# surfaces on develop after merge. The `test` job below runs BOTH suites via
|
||||
# `pnpm -r test` on every PR; the dedicated `mcp-server-parity` job makes that
|
||||
# cross-package gate explicit and fast. Do not add a `paths:` filter here.
|
||||
on:
|
||||
pull_request:
|
||||
workflow_call:
|
||||
@@ -140,53 +132,3 @@ jobs:
|
||||
# isolated `docmost_test` DB and migrates it to latest.
|
||||
- name: Run server integration tests
|
||||
run: pnpm --filter server test:int
|
||||
|
||||
# Cross-package tool-spec parity gate (issue #447). The tool-spec registry lives
|
||||
# in `packages/mcp` but its parity/tier guard tests live in the `apps/server`
|
||||
# jest suite, so a PR touching ONLY one of the two packages must still run BOTH
|
||||
# sides — otherwise an in-app wiring break (e.g. PR #434 drawio) passes the mcp
|
||||
# suite green and only surfaces on develop after merge. The `test` job already
|
||||
# runs everything via `pnpm -r test`; this job is a fast, explicitly-named guard
|
||||
# that runs the mcp `node --test` suite AND the server tool-guard jest specs
|
||||
# together, so the coupling is visible and can never be accidentally split by a
|
||||
# path filter. No Postgres/Redis needed: these specs mock the DB/loader.
|
||||
mcp-server-parity:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 15
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
|
||||
- name: Set up pnpm
|
||||
uses: pnpm/action-setup@v4
|
||||
|
||||
- name: Set up Node
|
||||
uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: 22
|
||||
cache: pnpm
|
||||
|
||||
- name: Install dependencies
|
||||
run: pnpm install --frozen-lockfile
|
||||
|
||||
# Shared deps first (build/ dirs are gitignored; see test.yml build order).
|
||||
- name: Build editor-ext
|
||||
run: pnpm --filter @docmost/editor-ext build
|
||||
|
||||
- name: Build prosemirror-markdown
|
||||
run: pnpm --filter @docmost/prosemirror-markdown build
|
||||
|
||||
# Build the mcp package so build/ carries a FRESH REGISTRY_STAMP (#447): the
|
||||
# build runs gen-registry-stamp.mjs before tsc, so a build/ vs src/ skew
|
||||
# cannot slip into the tests that exercise the loader's stale-check.
|
||||
- name: Build mcp (regenerates REGISTRY_STAMP)
|
||||
run: pnpm --filter @docmost/mcp build
|
||||
|
||||
# mcp side: the standalone MCP server's own tool-spec / instructions guards.
|
||||
- name: Run mcp tool-spec suite
|
||||
run: pnpm --filter @docmost/mcp test
|
||||
|
||||
# server side: the parity + tier guards that read packages/mcp/src/tool-specs
|
||||
# and assert the in-app AI-chat wiring matches it.
|
||||
- name: Run server tool-spec guard specs
|
||||
run: pnpm --filter server exec jest shared-tool-specs.contract tool-tiers ai-chat-tools.service --runInBand
|
||||
|
||||
@@ -19,11 +19,6 @@ packages/prosemirror-markdown/build/
|
||||
# markdown convention; the package is private and rebuilt at deploy.
|
||||
packages/mcp/build/
|
||||
|
||||
# mcp REGISTRY_STAMP codegen output (issue #447). Regenerated into src/ by
|
||||
# scripts/gen-registry-stamp.mjs on every `build`/`pretest` (before tsc), so it
|
||||
# is a build artifact like build/ — never committed, always fresh.
|
||||
packages/mcp/src/registry-stamp.generated.ts
|
||||
|
||||
# Logs
|
||||
logs
|
||||
*.log
|
||||
|
||||
@@ -248,22 +248,6 @@ pnpm collab:dev # run the collaboration server process standalone (
|
||||
> that order). Reach for it whenever you run a consumer package's checks on their
|
||||
> own rather than through the full `pnpm build`.
|
||||
|
||||
> **Editing an MCP tool spec requires a rebuild (issue #447).** The running
|
||||
> server loads the **compiled** `packages/mcp/build/` of `@docmost/mcp` (via the
|
||||
> runtime loader in `apps/server/src/core/ai-chat/tools/docmost-client.loader.ts`),
|
||||
> but the parity/tier guard tests read `packages/mcp/src/tool-specs.ts`. So if you
|
||||
> edit `tool-specs.ts` (any tool name, description, tier, catalog line, or input
|
||||
> schema) **without rebuilding**, `build/` and `src/` silently diverge — the tests
|
||||
> stay green while the server serves the OLD tools. To close that gap, the build
|
||||
> emits a `REGISTRY_STAMP` (a deterministic hash of the tool-specs content, via
|
||||
> `scripts/gen-registry-stamp.mjs` before `tsc`); on dev/test startup the loader
|
||||
> recomputes it from `src/` and **refuses to start with a "@docmost/mcp build is
|
||||
> stale …" error** on a mismatch (a pure no-op in prod, where only `build/` ships).
|
||||
> After editing tool specs, rebuild:
|
||||
> ```bash
|
||||
> pnpm --filter @docmost/mcp build # or: pnpm --filter @docmost/mcp watch
|
||||
> ```
|
||||
|
||||
**Lint** (per package — there is no root lint script):
|
||||
```bash
|
||||
pnpm --filter server lint # eslint --fix on server .ts
|
||||
|
||||
@@ -1,173 +0,0 @@
|
||||
import { createHash } from 'node:crypto';
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
|
||||
import { computeSrcRegistryStamp } from './docmost-client.loader';
|
||||
|
||||
// The exact message the loader throws on a build/src skew (issue #447). Kept as a
|
||||
// literal here so a reworded prod message reddens this test (the message is a
|
||||
// developer-facing contract: it tells them how to fix it).
|
||||
const STALE_BUILD_MESSAGE =
|
||||
'@docmost/mcp build is stale (tool-specs changed since last build) — run: pnpm --filter @docmost/mcp build';
|
||||
|
||||
// Replica of the loader's inline stale-check predicate + throw from
|
||||
// `loadDocmostMcp`. That guard is not independently exported (it lives inside the
|
||||
// dynamic-import IIFE, wired to a fixed `require.resolve('@docmost/mcp')`), so we
|
||||
// exercise the exact same three-condition logic against a stamp produced by the
|
||||
// REAL `computeSrcRegistryStamp`. This documents and locks the throw/no-throw
|
||||
// behaviour; if the prod predicate changes, this replica must change with it.
|
||||
function assertStaleGuard(
|
||||
srcStamp: string | null,
|
||||
registryStamp: string | undefined,
|
||||
): void {
|
||||
if (
|
||||
srcStamp !== null &&
|
||||
typeof registryStamp === 'string' &&
|
||||
srcStamp !== registryStamp
|
||||
) {
|
||||
throw new Error(STALE_BUILD_MESSAGE);
|
||||
}
|
||||
}
|
||||
|
||||
// Build a throwaway `<pkg>/build/index.js` + optional `<pkg>/src/tool-specs.ts`
|
||||
// layout so `computeSrcRegistryStamp(<pkg>/build/index.js)` resolves src the same
|
||||
// way the loader does (dirname(dirname(entry))/src/tool-specs.ts).
|
||||
function makeFakePackage(toolSpecsSource: string | null): {
|
||||
entry: string;
|
||||
cleanup: () => void;
|
||||
} {
|
||||
const root = mkdtempSync(join(tmpdir(), 'mcp-stamp-'));
|
||||
const buildDir = join(root, 'build');
|
||||
mkdirSync(buildDir, { recursive: true });
|
||||
const entry = join(buildDir, 'index.js');
|
||||
writeFileSync(entry, '// fake @docmost/mcp build entry\n', 'utf8');
|
||||
if (toolSpecsSource !== null) {
|
||||
const srcDir = join(root, 'src');
|
||||
mkdirSync(srcDir, { recursive: true });
|
||||
writeFileSync(join(srcDir, 'tool-specs.ts'), toolSpecsSource, 'utf8');
|
||||
}
|
||||
return { entry, cleanup: () => rmSync(root, { recursive: true, force: true }) };
|
||||
}
|
||||
|
||||
describe('computeSrcRegistryStamp (#447 stale-build guard)', () => {
|
||||
it('returns null when src/tool-specs.ts is absent (prod no-op path)', () => {
|
||||
// A prod image ships only build/, no src/ — the guard must be a silent no-op.
|
||||
const { entry, cleanup } = makeFakePackage(null);
|
||||
try {
|
||||
expect(computeSrcRegistryStamp(entry)).toBeNull();
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it('returns null for a bogus package entry (swallowed error path)', () => {
|
||||
// A resolution/read hiccup must NEVER break startup — it resolves to null.
|
||||
expect(
|
||||
computeSrcRegistryStamp('/no/such/pkg/build/index.js'),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('computes a 64-char sha256 hex when src/tool-specs.ts exists', () => {
|
||||
const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
|
||||
try {
|
||||
const stamp = computeSrcRegistryStamp(entry);
|
||||
expect(stamp).toMatch(/^[0-9a-f]{64}$/);
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it('normalizes CRLF->LF and strips a single trailing newline', () => {
|
||||
// A CRLF+trailing-newline variant of the same content hashes identically to
|
||||
// the bare-LF form — the guard must not fire on a checkout-style difference.
|
||||
const bare = makeFakePackage('alpha\nbeta');
|
||||
const crlfTrailing = makeFakePackage('alpha\r\nbeta\r\n');
|
||||
try {
|
||||
expect(computeSrcRegistryStamp(crlfTrailing.entry)).toBe(
|
||||
computeSrcRegistryStamp(bare.entry),
|
||||
);
|
||||
} finally {
|
||||
bare.cleanup();
|
||||
crlfTrailing.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
// CROSS-IMPL EQUALITY (covers reviewer suggestion 2). The SAME fixed input and
|
||||
// EXPECTED hash are asserted in the mcp-side node test
|
||||
// (packages/mcp/test/unit/registry-stamp.test.mjs) against the codegen's
|
||||
// `computeRegistryStamp`. Asserting the SAME pair here against the loader's
|
||||
// `computeSrcRegistryStamp` proves both implementations normalize+hash
|
||||
// identically; a divergence in EITHER side reddens one of the two tests.
|
||||
it('matches the documented cross-impl hash for a fixed input', () => {
|
||||
const FIXED_INPUT = 'line1\r\nline2\n';
|
||||
const EXPECTED =
|
||||
'683376e290829b482c2655745caffa7a1dccfa10afaa62dac2b42dd6c68d0f83';
|
||||
const { entry, cleanup } = makeFakePackage(FIXED_INPUT);
|
||||
try {
|
||||
expect(computeSrcRegistryStamp(entry)).toBe(EXPECTED);
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it('the documented EXPECTED is the normalize+sha256 of the fixed input', () => {
|
||||
// Proves EXPECTED is not a magic constant but the documented computation.
|
||||
const FIXED_INPUT = 'line1\r\nline2\n';
|
||||
const normalized = FIXED_INPUT.replace(/\r\n/g, '\n').replace(/\n$/, '');
|
||||
const expected = createHash('sha256')
|
||||
.update(normalized, 'utf8')
|
||||
.digest('hex');
|
||||
const { entry, cleanup } = makeFakePackage(FIXED_INPUT);
|
||||
try {
|
||||
expect(computeSrcRegistryStamp(entry)).toBe(expected);
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadDocmostMcp stale-check predicate (#447)', () => {
|
||||
it('THROWS the exact stale message when src stamp != built REGISTRY_STAMP', () => {
|
||||
const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
|
||||
try {
|
||||
const srcStamp = computeSrcRegistryStamp(entry);
|
||||
expect(srcStamp).not.toBeNull();
|
||||
// Simulate a stale build: build/ carries a DIFFERENT stamp than src.
|
||||
expect(() => assertStaleGuard(srcStamp, 'a'.repeat(64))).toThrow(
|
||||
STALE_BUILD_MESSAGE,
|
||||
);
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it('does NOT throw when src stamp equals the built REGISTRY_STAMP', () => {
|
||||
const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
|
||||
try {
|
||||
const srcStamp = computeSrcRegistryStamp(entry);
|
||||
// Fresh build: build/ stamp == src stamp -> guard is a no-op.
|
||||
expect(() => assertStaleGuard(srcStamp, srcStamp as string)).not.toThrow();
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it('does NOT throw when src is absent (prod: srcStamp === null)', () => {
|
||||
// Even against a present-but-mismatched REGISTRY_STAMP, a null src stamp
|
||||
// (prod image with build/ only) must skip the check entirely.
|
||||
expect(() => assertStaleGuard(null, 'a'.repeat(64))).not.toThrow();
|
||||
});
|
||||
|
||||
it('does NOT throw when REGISTRY_STAMP is absent (pre-#447 build)', () => {
|
||||
// An older @docmost/mcp build has no REGISTRY_STAMP export; the guard must be
|
||||
// a no-op so an out-of-date build never wrongly blocks startup.
|
||||
const { entry, cleanup } = makeFakePackage('export const specs = 1;\n');
|
||||
try {
|
||||
const srcStamp = computeSrcRegistryStamp(entry);
|
||||
expect(() => assertStaleGuard(srcStamp, undefined)).not.toThrow();
|
||||
} finally {
|
||||
cleanup();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,3 @@
|
||||
import { createHash } from 'node:crypto';
|
||||
import { existsSync, readFileSync } from 'node:fs';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
|
||||
/**
|
||||
@@ -347,50 +344,6 @@ interface DocmostMcpModule {
|
||||
// loader in unit tests. The in-app layer treats an absent factory as "signal
|
||||
// disabled" — a pure no-op that leaves tool results byte-identical.
|
||||
createCommentSignalTracker?: CommentSignalTrackerFactory;
|
||||
// Optional (#447): a deterministic hash of the tool-specs registry content,
|
||||
// generated into build/ by the package's build. Absent on a pre-#447 build (or
|
||||
// the mocked loader in unit tests) — the stale-check below is a NO-OP when it
|
||||
// is missing, so an older build never wrongly fails startup.
|
||||
REGISTRY_STAMP?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Recompute the REGISTRY_STAMP (#447) from the @docmost/mcp source tree, if it is
|
||||
* present. Returns the stamp string, or `null` when the source is absent (a prod
|
||||
* image ships only build/, no src/). MUST stay byte-for-byte identical to
|
||||
* packages/mcp/scripts/gen-registry-stamp.mjs's `computeRegistryStamp` so the
|
||||
* build-time and src-time hashes agree: same input file (src/tool-specs.ts), same
|
||||
* normalization (CRLF -> LF, strip a single trailing newline), same sha256.
|
||||
*
|
||||
* DEV vs PROD detection is by FILE EXISTENCE, not NODE_ENV: we resolve the
|
||||
* package's own directory from `require.resolve('@docmost/mcp')` (which points at
|
||||
* build/index.js) and look for ../src/tool-specs.ts next to it. In a dev/test
|
||||
* worktree that file exists; in a prod image (build/ only, src/ stripped) it does
|
||||
* not, so this returns null and the caller skips the check. Any error (ENOENT, a
|
||||
* bad resolve) is swallowed to null — the stale-check must NEVER break startup.
|
||||
*
|
||||
* Exported for unit testing (docmost-client.loader.spec.ts): the export keyword
|
||||
* is behaviourally a no-op — the module-internal caller `loadDocmostMcp` is
|
||||
* unaffected. The test drives the null (no-src) path and asserts this
|
||||
* normalize+sha256 stays identical to the codegen's `computeRegistryStamp`.
|
||||
*/
|
||||
export function computeSrcRegistryStamp(packageEntry: string): string | null {
|
||||
try {
|
||||
// packageEntry is <pkg>/build/index.js; the source lives at <pkg>/src/.
|
||||
const toolSpecsPath = join(
|
||||
dirname(dirname(packageEntry)),
|
||||
'src',
|
||||
'tool-specs.ts',
|
||||
);
|
||||
if (!existsSync(toolSpecsPath)) return null; // prod: no src tree -> skip.
|
||||
const source = readFileSync(toolSpecsPath, 'utf8');
|
||||
const normalized = source.replace(/\r\n/g, '\n').replace(/\n$/, '');
|
||||
return createHash('sha256').update(normalized, 'utf8').digest('hex');
|
||||
} catch {
|
||||
// Never let a resolution/read hiccup break server startup — treat as "no
|
||||
// src available" and skip the check (identical to the prod no-op path).
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
// TS with module:commonjs downlevels a literal `import()` to `require()`, which
|
||||
@@ -422,23 +375,6 @@ export async function loadDocmostMcp(): Promise<{
|
||||
const mod = (await esmImport(
|
||||
pathToFileURL(entry).href,
|
||||
)) as DocmostMcpModule;
|
||||
// #447 stale-build guard (dev/test only). The server loads the COMPILED
|
||||
// build/ of @docmost/mcp, but the parity/tier guard tests read src/. If a
|
||||
// tool spec is edited in src without rebuilding the package, build/ and src/
|
||||
// silently diverge and the running server serves the OLD tools. Here we
|
||||
// recompute the stamp from src/tool-specs.ts and compare it to the stamp
|
||||
// baked into build/. In PROD the src tree is absent (image ships build/
|
||||
// only), so computeSrcRegistryStamp returns null and this is a pure no-op.
|
||||
const srcStamp = computeSrcRegistryStamp(entry);
|
||||
if (
|
||||
srcStamp !== null &&
|
||||
typeof mod.REGISTRY_STAMP === 'string' &&
|
||||
srcStamp !== mod.REGISTRY_STAMP
|
||||
) {
|
||||
throw new Error(
|
||||
'@docmost/mcp build is stale (tool-specs changed since last build) — run: pnpm --filter @docmost/mcp build',
|
||||
);
|
||||
}
|
||||
return mod;
|
||||
})().catch((err) => {
|
||||
// Do not cache a rejected import — allow the next call to retry.
|
||||
|
||||
@@ -13,11 +13,10 @@
|
||||
"docmost-mcp": "./build/stdio.js"
|
||||
},
|
||||
"scripts": {
|
||||
"gen:stamp": "node scripts/gen-registry-stamp.mjs",
|
||||
"build": "node scripts/gen-registry-stamp.mjs && tsc",
|
||||
"build": "tsc",
|
||||
"start": "node build/stdio.js",
|
||||
"watch": "node scripts/gen-registry-stamp.mjs && tsc --watch",
|
||||
"pretest": "node scripts/gen-registry-stamp.mjs && tsc",
|
||||
"watch": "tsc --watch",
|
||||
"pretest": "tsc",
|
||||
"test": "node --test \"test/unit/*.test.mjs\" \"test/mock/*.test.mjs\"",
|
||||
"test:unit": "node --test \"test/unit/*.test.mjs\"",
|
||||
"test:mock": "node --test \"test/mock/*.test.mjs\"",
|
||||
|
||||
@@ -1,67 +0,0 @@
|
||||
// Codegen: emit src/registry-stamp.generated.ts with a REGISTRY_STAMP hash of
|
||||
// the tool-specs REGISTRY CONTENT, so a build/ vs src/ skew (issue #447) is
|
||||
// detectable at runtime.
|
||||
//
|
||||
// WHY hash the raw source text (not extracted structured data):
|
||||
// SHARED_TOOL_SPECS carries `buildShape` functions (the input SCHEMAS) which are
|
||||
// NOT serializable. The input schema is exactly one of the things that MUST stay
|
||||
// in sync between build/ and src/, so we cannot drop it from the hash. Rather
|
||||
// than probe zod with a fragile shim to reconstruct the schema shape, we hash the
|
||||
// STABLE, deterministic source TEXT of tool-specs.ts. That text fully captures
|
||||
// every field that must stay in sync — mcpName, inAppKey, description, tier,
|
||||
// catalogLine AND the buildShape bodies (input schemas) — with zero probing
|
||||
// fragility. Any edit to a spec (a renamed tool, a reworded description, a
|
||||
// changed schema field) changes the text and therefore the stamp.
|
||||
//
|
||||
// DETERMINISM: the hash is computed over the file bytes with line endings
|
||||
// normalized to LF and a single trailing newline stripped, so a CRLF checkout or
|
||||
// an editor's trailing-newline habit cannot make build/ and src/ disagree. No
|
||||
// Date.now / randomness. The loader's dev-only stale-check (docmost-client.loader.ts)
|
||||
// re-runs THIS SAME normalization + sha256 over src/tool-specs.ts and compares to
|
||||
// the built REGISTRY_STAMP; the two must compute identically.
|
||||
//
|
||||
// This script runs from the `build` and `pretest` npm scripts BEFORE tsc, so
|
||||
// build/ always carries a stamp derived from the tool-specs.ts that was compiled.
|
||||
|
||||
import { createHash } from 'node:crypto';
|
||||
import { readFileSync, writeFileSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { dirname, join } from 'node:path';
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const SRC_DIR = join(__dirname, '..', 'src');
|
||||
const TOOL_SPECS_PATH = join(SRC_DIR, 'tool-specs.ts');
|
||||
const OUT_PATH = join(SRC_DIR, 'registry-stamp.generated.ts');
|
||||
|
||||
/**
|
||||
* Deterministic stamp of the tool-specs registry content. Kept as a plain
|
||||
* function (exported) so the algorithm has a single home; the loader duplicates
|
||||
* only the tiny normalize+sha256 steps because it lives in the CJS server build
|
||||
* and cannot import this ESM script. If you change the normalization here, mirror
|
||||
* it in apps/server/src/core/ai-chat/tools/docmost-client.loader.ts.
|
||||
*/
|
||||
export function computeRegistryStamp(toolSpecsSource) {
|
||||
const normalized = toolSpecsSource.replace(/\r\n/g, '\n').replace(/\n$/, '');
|
||||
return createHash('sha256').update(normalized, 'utf8').digest('hex');
|
||||
}
|
||||
|
||||
function main() {
|
||||
const source = readFileSync(TOOL_SPECS_PATH, 'utf8');
|
||||
const stamp = computeRegistryStamp(source);
|
||||
const out =
|
||||
'// AUTO-GENERATED by scripts/gen-registry-stamp.mjs — DO NOT EDIT BY HAND.\n' +
|
||||
'// A deterministic hash of src/tool-specs.ts content (tool names, descriptions,\n' +
|
||||
'// tiers, catalog lines and input schemas). Regenerated on every build/pretest\n' +
|
||||
'// so build/ always matches the compiled src. The in-app loader recomputes this\n' +
|
||||
'// from src and refuses to run on a mismatch (issue #447). This file is\n' +
|
||||
'// gitignored and produced by the build — see .gitignore.\n' +
|
||||
`export const REGISTRY_STAMP = ${JSON.stringify(stamp)};\n`;
|
||||
writeFileSync(OUT_PATH, out, 'utf8');
|
||||
// eslint-disable-next-line no-console
|
||||
console.log(`gen-registry-stamp: wrote ${OUT_PATH} (${stamp.slice(0, 12)}…)`);
|
||||
}
|
||||
|
||||
// Only run when invoked directly (not when imported for computeRegistryStamp).
|
||||
if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) {
|
||||
main();
|
||||
}
|
||||
@@ -42,6 +42,7 @@ import {
|
||||
insertTableRow,
|
||||
deleteTableRow,
|
||||
updateTableCell,
|
||||
findInvalidNode,
|
||||
} from "@docmost/prosemirror-markdown";
|
||||
import { searchInDoc, SearchOptions } from "./lib/page-search.js";
|
||||
import { withPageLock } from "./lib/page-lock.js";
|
||||
@@ -1660,6 +1661,27 @@ export class DocmostClient {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Pre-write SHAPE gate (#409). Walk the WHOLE node tree with the shared
|
||||
* `findInvalidNode` and throw a rich, path-anchored error the instant a nested
|
||||
* node has an absent/unknown `type` (or an unknown mark) — the exact shape that
|
||||
* otherwise surfaces DEEP in the Yjs encode as the cryptic
|
||||
* `Unknown node type: undefined`, but only AFTER a collab session was opened
|
||||
* and a page lock taken. Calling this BEFORE `getCollabTokenWithReauth` /
|
||||
* `mutatePageContent` fails fast: no collab connection, no lock, deterministic
|
||||
* message. `op` names the tool for the message prefix (e.g. "patch_node").
|
||||
*
|
||||
* `findInvalidNode` derives its "known type" set from the very same
|
||||
* `docmostExtensions` the encode path uses, so a node this gate accepts is one
|
||||
* the encoder will accept too.
|
||||
*/
|
||||
private assertValidNodeShape(op: string, node: any): void {
|
||||
const bad = findInvalidNode(node);
|
||||
if (bad) {
|
||||
throw new Error(`${op}: invalid node — ${bad.summary}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Replace page content with a raw ProseMirror JSON document (lossless) and/or
|
||||
* update its title. Both `doc` and `title` are optional, but at least one must
|
||||
@@ -1711,6 +1733,12 @@ export class DocmostClient {
|
||||
// page on overwrite.
|
||||
this.validateDocStructure(doc);
|
||||
|
||||
// #409: beyond the string-`type` check above, reject a nested node whose
|
||||
// `type` is a string but NOT a known Docmost schema node (a typo/unknown
|
||||
// block) — the same `Unknown node type` the encoder throws — with a rich,
|
||||
// path-anchored message, still BEFORE any collab connection.
|
||||
this.assertValidNodeShape("update_page_json", doc);
|
||||
|
||||
// Sanitize URLs before writing. This closes the JSON-path bypass: unlike
|
||||
// the markdown link path (which TipTap sanitizes), raw JSON could otherwise
|
||||
// inject javascript:/data: link hrefs or media srcs straight into the doc.
|
||||
@@ -2131,6 +2159,14 @@ export class DocmostClient {
|
||||
target.attrs.id = nodeId;
|
||||
}
|
||||
|
||||
// #409: fail fast on a malformed node SHAPE (a nested child with an
|
||||
// absent/unknown `type`, e.g. a text leaf written as `{"text":"foo"}` with
|
||||
// no `"type":"text"`) BEFORE opening a collab session or taking the page
|
||||
// lock — the root-only `typeof node.type === "string"` check above never
|
||||
// sees nested children, and the encoder's `Unknown node type: undefined`
|
||||
// would otherwise only surface after the connection.
|
||||
this.assertValidNodeShape("patch_node", target);
|
||||
|
||||
const collabToken = await this.getCollabTokenWithReauth();
|
||||
// Open the collab doc by the canonical UUID, never the slugId (#260).
|
||||
const pageUuid = await this.resolvePageId(pageId);
|
||||
@@ -2221,6 +2257,11 @@ export class DocmostClient {
|
||||
}
|
||||
}
|
||||
|
||||
// #409: fail fast on a malformed node SHAPE (a nested child with an
|
||||
// absent/unknown `type`) BEFORE opening a collab session or taking the page
|
||||
// lock — the root-only check above never sees nested children.
|
||||
this.assertValidNodeShape("insert_node", node);
|
||||
|
||||
const collabToken = await this.getCollabTokenWithReauth();
|
||||
// Open the collab doc by the canonical UUID, never the slugId (#260).
|
||||
const pageUuid = await this.resolvePageId(pageId);
|
||||
|
||||
@@ -29,14 +29,6 @@ export { destroyAllSessions } from "./lib/collab-session.js";
|
||||
export { SHARED_TOOL_SPECS } from "./tool-specs.js";
|
||||
export type { SharedToolSpec } from "./tool-specs.js";
|
||||
|
||||
// Re-export the build-time REGISTRY_STAMP (issue #447): a deterministic hash of
|
||||
// the tool-specs registry content, generated into src/registry-stamp.generated.ts
|
||||
// by scripts/gen-registry-stamp.mjs BEFORE tsc, so it lands in build/. The in-app
|
||||
// loader recomputes the same hash from src/tool-specs.ts (dev/test only) and
|
||||
// refuses to run on a mismatch, catching a build/ vs src/ skew (a spec edited in
|
||||
// src without rebuilding the package the server actually loads from build/).
|
||||
export { REGISTRY_STAMP } from "./registry-stamp.generated.js";
|
||||
|
||||
// Re-export the shared "new comments: N" signal helper (#417) so the in-app
|
||||
// layer reads the SAME watermark/debounce/injection-safe line builder off the
|
||||
// loaded module (same pattern as SHARED_TOOL_SPECS). Both surfaces then differ
|
||||
|
||||
@@ -13,7 +13,11 @@ import { JSDOM } from "jsdom";
|
||||
import { markdownToProseMirror } from "@docmost/prosemirror-markdown";
|
||||
import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
|
||||
import { withPageLock } from "./page-lock.js";
|
||||
import { sanitizeForYjs, findUnstorableAttr } from "@docmost/prosemirror-markdown";
|
||||
import {
|
||||
sanitizeForYjs,
|
||||
findUnstorableAttr,
|
||||
findInvalidNode,
|
||||
} from "@docmost/prosemirror-markdown";
|
||||
import { canonicalizeFootnotes } from "./footnote-canonicalize.js";
|
||||
import { normalizeAndMergeFootnotes } from "./footnote-normalize-merge.js";
|
||||
import { VerifyReport } from "./diff.js";
|
||||
@@ -28,11 +32,25 @@ export { markdownToProseMirror };
|
||||
* place. `label` names the stage that failed (diagnostic). `sanitizeForYjs`
|
||||
* already stripped `undefined` attrs, so a remaining failure is pinpointed via
|
||||
* `findUnstorableAttr`.
|
||||
*
|
||||
* Diagnostics precedence (#409): the dominant crash here is
|
||||
* `Unknown node type: undefined` — a nested node with an absent/unknown `type`
|
||||
* (a SHAPE problem, e.g. `{"text":"foo"}` missing `"type":"text"`). That points
|
||||
* at the node, not an attribute, so `findInvalidNode` is consulted FIRST and,
|
||||
* on a hit, yields a path-anchored node-shape message. Only when the document
|
||||
* shape is sound do we fall back to `findUnstorableAttr` (undefined/function/
|
||||
* symbol/bigint attr values); the generic "attribute likely holds a value Yjs
|
||||
* cannot store" sentence is the last resort.
|
||||
*/
|
||||
function unstorableYjsError(safe: any, label: string, e: unknown): Error {
|
||||
const base = `Failed to encode document to Yjs (${label}): ${e instanceof Error ? e.message : String(e)}.`;
|
||||
const badNode = findInvalidNode(safe);
|
||||
if (badNode) {
|
||||
return new Error(`${base} Invalid node: ${badNode.summary}`);
|
||||
}
|
||||
const bad = findUnstorableAttr(safe);
|
||||
return new Error(
|
||||
`Failed to encode document to Yjs (${label}): ${e instanceof Error ? e.message : String(e)}.${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`,
|
||||
`${base}${bad ? ` Offending attribute: ${bad}.` : " A node/mark attribute likely holds a value Yjs cannot store (e.g. undefined)."}`,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -236,7 +236,10 @@ export const SHARED_TOOL_SPECS = {
|
||||
'heading {"type":"heading","attrs":{"level":2},"content":' +
|
||||
'[{"type":"text","text":"Title"}]}. Bold is a mark: ' +
|
||||
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' +
|
||||
'JSON object or a JSON string (both accepted). Cheaper and safer than ' +
|
||||
'JSON object or a JSON string (both accepted). EVERY node, including ' +
|
||||
'nested children, must carry a string `type` from the Docmost schema; ' +
|
||||
'text leaves are {"type":"text","text":"..."} (a bare {"text":"..."} is ' +
|
||||
'rejected up front). Cheaper and safer than ' +
|
||||
'replacing the whole document for one-block structural edits. Reversible: ' +
|
||||
'the previous version is kept in page history.',
|
||||
tier: 'deferred',
|
||||
@@ -282,7 +285,10 @@ export const SHARED_TOOL_SPECS = {
|
||||
'{"type":"paragraph","content":[{"type":"text","text":"Hello"}]} or a ' +
|
||||
'heading {"type":"heading","attrs":{"level":2},"content":' +
|
||||
'[{"type":"text","text":"Title"}]}. Bold is a mark: ' +
|
||||
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' +
|
||||
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. EVERY node, ' +
|
||||
'including nested children, must carry a string `type` from the Docmost ' +
|
||||
'schema; text leaves are {"type":"text","text":"..."} (a bare ' +
|
||||
'{"text":"..."} is rejected up front). The node may be a ' +
|
||||
'JSON object or a JSON string (both accepted). Reversible via page history.',
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
@@ -705,7 +711,10 @@ export const SHARED_TOOL_SPECS = {
|
||||
'"paragraph","content":[{"type":"text","text":"Hi"}]}]}. `content` may be ' +
|
||||
'a JSON object or a JSON string (both accepted), and is OPTIONAL: omit it ' +
|
||||
'to update only the title (though prefer the rename-page tool for a title-only ' +
|
||||
'change). Supplying neither content nor title is an error. Reversible: ' +
|
||||
'change). Supplying neither content nor title is an error. EVERY node, ' +
|
||||
'including nested children, must carry a string `type` from the Docmost ' +
|
||||
'schema; text leaves are {"type":"text","text":"..."} (a bare ' +
|
||||
'{"text":"..."} is rejected up front). Reversible: ' +
|
||||
'the previous version is kept in page history.',
|
||||
tier: 'deferred',
|
||||
catalogLine:
|
||||
|
||||
@@ -0,0 +1,240 @@
|
||||
// Mock regression for the FAIL-FAST invalid-node validation (#409).
|
||||
//
|
||||
// A structural editor (patch_node / insert_node / update_page_json) given a doc
|
||||
// whose NESTED child has an absent/unknown `type` (the exact shape the Yjs
|
||||
// encoder rejects with `Unknown node type: undefined`) must throw a RICH,
|
||||
// path-anchored error BEFORE it ever opens a collab session or takes a page
|
||||
// lock. We prove the fail-fast by standing up a collab stack whose HTTP handler
|
||||
// records EVERY request: a correct fail-fast never even fetches the collab
|
||||
// token (which `getCollabTokenWithReauth`, called AFTER the validation, would
|
||||
// request), and never drives a document change on the Hocuspocus doc.
|
||||
//
|
||||
// The happy path (a well-formed doc) is exercised too: it must reach the collab
|
||||
// write and succeed, so the gate is not over-eager.
|
||||
//
|
||||
// findInvalidNode's per-shape summaries are unit-tested in the package
|
||||
// (test/find-invalid-node.test.ts); this exercises the END-TO-END wiring through
|
||||
// the real client methods.
|
||||
import { test, after } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import http from "node:http";
|
||||
import { WebSocketServer } from "ws";
|
||||
import { Hocuspocus } from "@hocuspocus/server";
|
||||
import { DocmostClient } from "../../build/client.js";
|
||||
import { buildYDoc } from "../../build/lib/collaboration.js";
|
||||
|
||||
// A minimal valid seed doc with a real block id, so the happy-path patch_node
|
||||
// finds its target.
|
||||
const SEED_ID = "seed-para-id";
|
||||
function seedDoc() {
|
||||
return {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
attrs: { id: SEED_ID },
|
||||
content: [{ type: "text", text: "seed" }],
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
// Stand up an HTTP server that authenticates + hands out a collab token AND
|
||||
// upgrades /collab to a Hocuspocus instance seeded with the doc. `state` records
|
||||
// whether the collab token was ever fetched (proving the write path was entered)
|
||||
// and whether the Hocuspocus doc ever changed.
|
||||
async function spawnCollabStack() {
|
||||
const state = { changed: false, collabTokenFetched: false };
|
||||
|
||||
const hocuspocus = new Hocuspocus({
|
||||
quiet: true,
|
||||
async onLoadDocument() {
|
||||
return buildYDoc(seedDoc());
|
||||
},
|
||||
async onChange() {
|
||||
state.changed = true;
|
||||
},
|
||||
});
|
||||
|
||||
const wss = new WebSocketServer({ noServer: true });
|
||||
|
||||
const server = http.createServer((req, res) => {
|
||||
let raw = "";
|
||||
req.on("data", (c) => (raw += c));
|
||||
req.on("end", () => {
|
||||
if (req.url === "/api/auth/login") {
|
||||
res.writeHead(200, {
|
||||
"Content-Type": "application/json",
|
||||
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
|
||||
});
|
||||
res.end(JSON.stringify({ success: true }));
|
||||
return;
|
||||
}
|
||||
if (req.url === "/api/auth/collab-token") {
|
||||
state.collabTokenFetched = true;
|
||||
res.writeHead(200, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ data: { token: "collab-jwt" } }));
|
||||
return;
|
||||
}
|
||||
res.writeHead(404, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ message: "not found" }));
|
||||
});
|
||||
});
|
||||
|
||||
server.on("upgrade", (request, socket, head) => {
|
||||
if (!request.url || !request.url.startsWith("/collab")) {
|
||||
socket.destroy();
|
||||
return;
|
||||
}
|
||||
wss.handleUpgrade(request, socket, head, (ws) => {
|
||||
hocuspocus.handleConnection(ws, request);
|
||||
});
|
||||
});
|
||||
|
||||
const baseURL = await new Promise((resolve) => {
|
||||
server.listen(0, "127.0.0.1", () => {
|
||||
const { port } = server.address();
|
||||
resolve(`http://127.0.0.1:${port}/api`);
|
||||
});
|
||||
});
|
||||
|
||||
openStacks.push({ server, hocuspocus });
|
||||
return { state, baseURL };
|
||||
}
|
||||
|
||||
const openStacks = [];
|
||||
after(async () => {
|
||||
await Promise.all(
|
||||
openStacks.map(
|
||||
({ server, hocuspocus }) =>
|
||||
new Promise((resolve) => {
|
||||
server.close(() => {
|
||||
Promise.resolve(hocuspocus.destroy?.()).finally(resolve);
|
||||
});
|
||||
}),
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
const PAGE = "11111111-1111-4111-8111-111111111111";
|
||||
|
||||
// A node whose NESTED text leaf is missing "type":"text" (dominant #409 shape).
|
||||
const nestedTypelessNode = () => ({
|
||||
type: "paragraph",
|
||||
content: [{ text: "oops", marks: [] }],
|
||||
});
|
||||
|
||||
// A node with a NESTED unknown type NAME (typo).
|
||||
const nestedUnknownTypeNode = () => ({
|
||||
type: "paragraph",
|
||||
content: [{ type: "paragraf", content: [{ type: "text", text: "x" }] }],
|
||||
});
|
||||
|
||||
test("patch_node fails fast on a nested typeless node — no collab connection", async () => {
|
||||
const { state, baseURL } = await spawnCollabStack();
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
|
||||
await assert.rejects(
|
||||
() => client.patchNode(PAGE, SEED_ID, nestedTypelessNode()),
|
||||
(err) => {
|
||||
assert.match(err.message, /patch_node: invalid node/);
|
||||
assert.match(err.message, /missing "type"/);
|
||||
assert.match(err.message, /content\[0\]/); // path-anchored
|
||||
return true;
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(
|
||||
state.collabTokenFetched,
|
||||
false,
|
||||
"must NOT fetch a collab token — validation runs before getCollabTokenWithReauth",
|
||||
);
|
||||
assert.equal(state.changed, false, "the collab doc must never be written");
|
||||
});
|
||||
|
||||
test("insert_node fails fast on a nested UNKNOWN type — no collab connection", async () => {
|
||||
const { state, baseURL } = await spawnCollabStack();
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
|
||||
await assert.rejects(
|
||||
() =>
|
||||
client.insertNode(PAGE, nestedUnknownTypeNode(), {
|
||||
position: "append",
|
||||
}),
|
||||
(err) => {
|
||||
assert.match(err.message, /insert_node: invalid node/);
|
||||
assert.match(err.message, /unknown node type "paragraf"/);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(state.collabTokenFetched, false);
|
||||
assert.equal(state.changed, false);
|
||||
});
|
||||
|
||||
test("update_page_json fails fast on a nested typeless node — no collab connection", async () => {
|
||||
const { state, baseURL } = await spawnCollabStack();
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
|
||||
const badDoc = {
|
||||
type: "doc",
|
||||
content: [{ type: "paragraph", content: [{ text: "oops" }] }],
|
||||
};
|
||||
|
||||
await assert.rejects(
|
||||
() => client.updatePageJson(PAGE, badDoc),
|
||||
(err) => {
|
||||
// update_page_json runs validateDocStructure first (string-type check),
|
||||
// which already rejects a typeless node — so the message may come from
|
||||
// either guard, but the write must not happen.
|
||||
assert.match(err.message, /type/i);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(state.collabTokenFetched, false);
|
||||
assert.equal(state.changed, false);
|
||||
});
|
||||
|
||||
test("update_page_json fails fast on a nested UNKNOWN type name — rich #409 message", async () => {
|
||||
const { state, baseURL } = await spawnCollabStack();
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
|
||||
// validateDocStructure passes (type is a string); assertValidNodeShape must
|
||||
// catch the unknown schema name and produce the rich path-anchored message.
|
||||
const badDoc = {
|
||||
type: "doc",
|
||||
content: [{ type: "paragraf", content: [{ type: "text", text: "x" }] }],
|
||||
};
|
||||
|
||||
await assert.rejects(
|
||||
() => client.updatePageJson(PAGE, badDoc),
|
||||
(err) => {
|
||||
assert.match(err.message, /update_page_json: invalid node/);
|
||||
assert.match(err.message, /unknown node type "paragraf"/);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(state.collabTokenFetched, false);
|
||||
assert.equal(state.changed, false);
|
||||
});
|
||||
|
||||
test("patch_node with a well-formed node proceeds to the collab write", async () => {
|
||||
const { state, baseURL } = await spawnCollabStack();
|
||||
const client = new DocmostClient(baseURL, "user@example.com", "pw");
|
||||
|
||||
const result = await client.patchNode(PAGE, SEED_ID, {
|
||||
type: "paragraph",
|
||||
content: [{ type: "text", text: "replacement" }],
|
||||
});
|
||||
|
||||
assert.equal(result.success, true);
|
||||
assert.equal(result.replaced, 1);
|
||||
assert.equal(
|
||||
state.collabTokenFetched,
|
||||
true,
|
||||
"a valid node must reach the collab write path",
|
||||
);
|
||||
assert.equal(state.changed, true, "the collab doc must be written");
|
||||
});
|
||||
@@ -1,101 +0,0 @@
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { createHash } from "node:crypto";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, join } from "node:path";
|
||||
|
||||
import { computeRegistryStamp } from "../../scripts/gen-registry-stamp.mjs";
|
||||
import { REGISTRY_STAMP } from "../../build/index.js";
|
||||
|
||||
// Guard tests for the build/src-skew stamp (issue #447). The codegen script
|
||||
// exports `computeRegistryStamp(sourceText)` — a sha256 over normalized source
|
||||
// text (CRLF->LF, single trailing newline stripped). The in-app loader
|
||||
// (apps/server/.../docmost-client.loader.ts) DUPLICATES that normalize+sha256 to
|
||||
// recompute the stamp from src and refuse a stale build. These tests pin the
|
||||
// algorithm's behaviour AND assert the built stamp matches the current src, so a
|
||||
// stale generated file OR a normalize divergence reddens.
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const TOOL_SPECS_PATH = join(__dirname, "..", "..", "src", "tool-specs.ts");
|
||||
|
||||
test("computeRegistryStamp is deterministic: same input -> same hash", () => {
|
||||
const input = "export const X = 1;\nexport const Y = 2;\n";
|
||||
assert.equal(computeRegistryStamp(input), computeRegistryStamp(input));
|
||||
});
|
||||
|
||||
test("computeRegistryStamp returns a 64-char lowercase hex sha256", () => {
|
||||
const stamp = computeRegistryStamp("anything");
|
||||
assert.match(stamp, /^[0-9a-f]{64}$/);
|
||||
});
|
||||
|
||||
test("normalizes CRLF vs LF: the same content hashes equal", () => {
|
||||
const lf = "line1\nline2\nline3";
|
||||
const crlf = "line1\r\nline2\r\nline3";
|
||||
assert.equal(computeRegistryStamp(crlf), computeRegistryStamp(lf));
|
||||
});
|
||||
|
||||
test("normalizes a trailing newline: with/without a final \\n hashes equal", () => {
|
||||
const noTrailing = "alpha\nbeta";
|
||||
const trailing = "alpha\nbeta\n";
|
||||
assert.equal(computeRegistryStamp(trailing), computeRegistryStamp(noTrailing));
|
||||
});
|
||||
|
||||
test("a CRLF checkout WITH a trailing CRLF still hashes equal to bare LF", () => {
|
||||
// A worst-case Windows checkout: CRLF line endings + a trailing CRLF. Both the
|
||||
// \r\n->\n replace and the trailing-newline strip must apply for parity.
|
||||
const bare = "alpha\nbeta";
|
||||
const crlfTrailing = "alpha\r\nbeta\r\n";
|
||||
assert.equal(
|
||||
computeRegistryStamp(crlfTrailing),
|
||||
computeRegistryStamp(bare),
|
||||
);
|
||||
});
|
||||
|
||||
test("a real content change hashes differently", () => {
|
||||
const before = "export const description = 'search a page';\n";
|
||||
const after = "export const description = 'search a PAGE';\n";
|
||||
assert.notEqual(computeRegistryStamp(before), computeRegistryStamp(after));
|
||||
});
|
||||
|
||||
// Only a SINGLE trailing newline is stripped — a second blank line is content and
|
||||
// must change the hash. This pins the exact `/\n$/` semantics the loader mirrors.
|
||||
test("only ONE trailing newline is stripped (two differ from one)", () => {
|
||||
assert.notEqual(
|
||||
computeRegistryStamp("x\n"),
|
||||
computeRegistryStamp("x\n\n"),
|
||||
);
|
||||
});
|
||||
|
||||
// Cross-impl equality against a fixed, documented input. The SAME literal input
|
||||
// and expected hash are asserted in the server-side jest test
|
||||
// (docmost-client.loader.spec.ts). If either side's normalize+sha256 ever
|
||||
// diverges, one of the two tests reddens. Input exercises BOTH normalize steps.
|
||||
test("fixed-input hash matches the documented cross-impl value", () => {
|
||||
const FIXED_INPUT = "line1\r\nline2\n";
|
||||
const EXPECTED =
|
||||
"683376e290829b482c2655745caffa7a1dccfa10afaa62dac2b42dd6c68d0f83";
|
||||
assert.equal(computeRegistryStamp(FIXED_INPUT), EXPECTED);
|
||||
});
|
||||
|
||||
// DESYNC GUARD (covers reviewer suggestion 2). Recompute the stamp from the
|
||||
// actual src/tool-specs.ts and assert it equals the REGISTRY_STAMP baked into the
|
||||
// freshly-built build/index.js. This reddens if the generated file is stale OR if
|
||||
// the codegen normalize ever diverges from what produced the built stamp.
|
||||
test("built REGISTRY_STAMP equals the stamp recomputed from src/tool-specs.ts", () => {
|
||||
const source = readFileSync(TOOL_SPECS_PATH, "utf8");
|
||||
assert.equal(computeRegistryStamp(source), REGISTRY_STAMP);
|
||||
});
|
||||
|
||||
// Sanity: the fixed-input helper computes the SAME way the codegen does, proving
|
||||
// the EXPECTED constant above is not an arbitrary magic value but the documented
|
||||
// normalize+sha256 of FIXED_INPUT. Belt-and-braces so a bad EXPECTED can't hide a
|
||||
// real regression.
|
||||
test("the documented EXPECTED constant is the normalize+sha256 of FIXED_INPUT", () => {
|
||||
const FIXED_INPUT = "line1\r\nline2\n";
|
||||
const normalized = FIXED_INPUT.replace(/\r\n/g, "\n").replace(/\n$/, "");
|
||||
const expected = createHash("sha256")
|
||||
.update(normalized, "utf8")
|
||||
.digest("hex");
|
||||
assert.equal(computeRegistryStamp(FIXED_INPUT), expected);
|
||||
});
|
||||
@@ -0,0 +1,82 @@
|
||||
// #409: unstorableYjsError diagnostics PRECEDENCE. The opaque Yjs encode failure
|
||||
// (`Unknown node type: undefined`) is a node-SHAPE problem, so the shared
|
||||
// findInvalidNode is consulted FIRST and yields a path-anchored node message;
|
||||
// only a shape-sound doc falls back to findUnstorableAttr (undefined/function/
|
||||
// etc. attr values). Exercised through `assertYjsEncodable`, which runs the same
|
||||
// encode + error-wrapping the live write path uses.
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { assertYjsEncodable } from "../../build/lib/collaboration.js";
|
||||
import {
|
||||
findInvalidNode,
|
||||
findUnstorableAttr,
|
||||
} from "@docmost/prosemirror-markdown";
|
||||
|
||||
const doc = (...content) => ({ type: "doc", content });
|
||||
|
||||
test("a nested typeless node yields the rich node-shape message (not an attr hint)", () => {
|
||||
const bad = doc({
|
||||
type: "paragraph",
|
||||
content: [{ text: "oops", marks: [] }], // missing "type":"text"
|
||||
});
|
||||
assert.throws(
|
||||
() => assertYjsEncodable(bad),
|
||||
(err) => {
|
||||
assert.match(err.message, /Invalid node:/);
|
||||
assert.match(err.message, /missing "type"/);
|
||||
assert.match(err.message, /content\[0\]/);
|
||||
// It must NOT fall through to the generic attribute sentence.
|
||||
assert.doesNotMatch(err.message, /Offending attribute/);
|
||||
assert.doesNotMatch(
|
||||
err.message,
|
||||
/attribute likely holds a value Yjs cannot store/,
|
||||
);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test("PRECEDENCE: a genuine undefined-attr case is a node-SHAPE-clean case, so the attr fallback fires", () => {
|
||||
// A doc whose node shapes are ALL valid but that carries a Yjs-unstorable
|
||||
// undefined attribute. unstorableYjsError checks findInvalidNode FIRST (must
|
||||
// miss here) and only then findUnstorableAttr (must hit) — this is exactly the
|
||||
// division of labor that keeps a real attr problem from being mislabelled as a
|
||||
// node-shape problem, and vice versa. We assert the two helpers directly (the
|
||||
// wrapper is not exported) because sanitizeForYjs strips undefined attrs before
|
||||
// the live encoder ever sees them, so this branch cannot be reached through
|
||||
// assertYjsEncodable without also failing the clone.
|
||||
const attrProblem = doc({
|
||||
type: "paragraph",
|
||||
attrs: { id: "p1", indent: undefined },
|
||||
content: [{ type: "text", text: "hi" }],
|
||||
});
|
||||
// findInvalidNode: shape is clean -> null (so the wrapper does NOT emit
|
||||
// "Invalid node").
|
||||
assert.equal(findInvalidNode(attrProblem), null);
|
||||
// findUnstorableAttr: pinpoints the undefined attr -> the fallback message.
|
||||
assert.match(findUnstorableAttr(attrProblem) ?? "", /indent \(undefined\)/);
|
||||
});
|
||||
|
||||
test("PRECEDENCE: a node-shape problem is caught by findInvalidNode even when an attr is also unstorable", () => {
|
||||
// Both a shape problem (typeless nested leaf) AND an unstorable attr exist;
|
||||
// findInvalidNode wins, so the model is pointed at the node shape (the real
|
||||
// root cause of `Unknown node type: undefined`), not the attribute.
|
||||
const both = doc({
|
||||
type: "paragraph",
|
||||
attrs: { id: "p1", indent: undefined },
|
||||
content: [{ text: "oops" }],
|
||||
});
|
||||
const shape = findInvalidNode(both);
|
||||
assert.notEqual(shape, null);
|
||||
assert.match(shape.summary, /missing "type"/);
|
||||
});
|
||||
|
||||
test("a fully valid document encodes without throwing", () => {
|
||||
const good = doc({
|
||||
type: "paragraph",
|
||||
attrs: { id: "p1" },
|
||||
content: [{ type: "text", text: "hi" }],
|
||||
});
|
||||
assert.doesNotThrow(() => assertYjsEncodable(good));
|
||||
});
|
||||
@@ -56,6 +56,7 @@ export {
|
||||
deleteNodeById,
|
||||
sanitizeForYjs,
|
||||
findUnstorableAttr,
|
||||
findInvalidNode,
|
||||
insertNodeRelative,
|
||||
readTable,
|
||||
insertTableRow,
|
||||
|
||||
@@ -14,7 +14,9 @@
|
||||
* `content`, non-object nodes, and absent `attrs` are tolerated.
|
||||
*/
|
||||
|
||||
import { getSchema } from "@tiptap/core";
|
||||
import { stripInlineMarkdown } from "./text-normalize.js";
|
||||
import { docmostExtensions } from "./docmost-schema.js";
|
||||
|
||||
/** Deep-clone a JSON-serializable value without mutating the original. */
|
||||
function clone<T>(value: T): T {
|
||||
@@ -383,6 +385,119 @@ export function findUnstorableAttr(doc: any): string | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The Docmost schema's known node and mark NAME sets, derived ONCE from the very
|
||||
* same `docmostExtensions` the Yjs encode path builds its schema from
|
||||
* (`getSchema(docmostExtensions)` — mirrored in mcp's `docmostSchema`). Deriving
|
||||
* both from the same extension list guarantees `findInvalidNode`'s "known type"
|
||||
* set matches exactly what `PMNode.fromJSON`/`toYdoc` will actually accept, so
|
||||
* the walker never flags a node the encoder would have stored (or vice versa).
|
||||
* Lazy + cached: the schema is only built on first use.
|
||||
*/
|
||||
let schemaNames: { nodes: Set<string>; marks: Set<string> } | null = null;
|
||||
function getSchemaNames(): { nodes: Set<string>; marks: Set<string> } {
|
||||
if (schemaNames == null) {
|
||||
const schema = getSchema(docmostExtensions);
|
||||
schemaNames = {
|
||||
nodes: new Set(Object.keys(schema.nodes)),
|
||||
marks: new Set(Object.keys(schema.marks)),
|
||||
};
|
||||
}
|
||||
return schemaNames;
|
||||
}
|
||||
|
||||
/**
|
||||
* Depth-first walk of the JSON `content` tree looking for the FIRST node whose
|
||||
* SHAPE the Yjs encode path will reject with an opaque
|
||||
* `Unknown node type: undefined` (issue #409). Returns `{ path, summary }` for
|
||||
* the offending node, or `null` when every node (and every mark) is a known
|
||||
* Docmost schema type.
|
||||
*
|
||||
* Two failure modes are detected, in order, per node:
|
||||
* 1. `type` is missing or not a string — the dominant `undefined` case, e.g.
|
||||
* a text leaf written as `{"text":"foo"}` with no `"type":"text"`.
|
||||
* 2. `type` is a string but NOT a known Docmost node name (a typo / unknown
|
||||
* block), OR one of the node's marks carries an unknown mark name.
|
||||
*
|
||||
* The returned `summary` is a model-actionable, path-anchored message such as:
|
||||
* `node.content[2].content[0]: missing "type" (keys: text, marks) — did you
|
||||
* mean {"type": "text", ...}?`
|
||||
* or for an unknown type:
|
||||
* `node.content[1]: unknown node type "paragraf" — not in the Docmost schema`
|
||||
*
|
||||
* `path` is the same dotted JSON path used in the summary (e.g.
|
||||
* `node.content[2].content[0]`) so callers can surface it separately. Null-safe:
|
||||
* a non-object doc returns `null`.
|
||||
*
|
||||
* NOTE: This is a SHAPE check, not a full ProseMirror content-model validation
|
||||
* (it does not verify that a paragraph may legally contain a table, etc.). Its
|
||||
* job is to turn the specific "unknown/absent node type" Yjs crash into a clear,
|
||||
* pre-write diagnostic; the schema's own `.check()` still catches deeper
|
||||
* content-model violations at encode time.
|
||||
*/
|
||||
export function findInvalidNode(
|
||||
doc: any,
|
||||
): { path: string; summary: string } | null {
|
||||
if (!isObject(doc)) return null;
|
||||
const { nodes, marks } = getSchemaNames();
|
||||
|
||||
// Build the "did you mean" hint for a typeless node from its own keys, so the
|
||||
// model sees WHICH object is malformed and the canonical text-leaf fix.
|
||||
const keyHint = (node: Record<string, any>): string => {
|
||||
const keys = Object.keys(node);
|
||||
const looksLikeText =
|
||||
typeof node.text === "string" && node.type === undefined;
|
||||
const suffix = looksLikeText
|
||||
? ` — did you mean {"type": "text", ...}?`
|
||||
: ` — every node needs a string "type" from the Docmost schema`;
|
||||
return `missing "type" (keys: ${keys.join(", ") || "none"})${suffix}`;
|
||||
};
|
||||
|
||||
const walk = (
|
||||
node: any,
|
||||
path: string,
|
||||
): { path: string; summary: string } | null => {
|
||||
if (!isObject(node)) return null;
|
||||
|
||||
// (1) missing / non-string type.
|
||||
if (typeof node.type !== "string") {
|
||||
return { path, summary: `${path}: ${keyHint(node)}` };
|
||||
}
|
||||
// (2) string type that is not a known Docmost node.
|
||||
if (!nodes.has(node.type)) {
|
||||
return {
|
||||
path,
|
||||
summary: `${path}: unknown node type "${node.type}" — not in the Docmost schema`,
|
||||
};
|
||||
}
|
||||
// (2b) unknown mark on an otherwise-valid node.
|
||||
if (Array.isArray(node.marks)) {
|
||||
for (let i = 0; i < node.marks.length; i++) {
|
||||
const mark = node.marks[i];
|
||||
if (isObject(mark) && typeof mark.type === "string" && !marks.has(mark.type)) {
|
||||
return {
|
||||
path: `${path}.marks[${i}]`,
|
||||
summary: `${path}.marks[${i}]: unknown mark type "${mark.type}" — not in the Docmost schema`,
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (Array.isArray(node.content)) {
|
||||
for (let i = 0; i < node.content.length; i++) {
|
||||
const hit = walk(node.content[i], `${path}.content[${i}]`);
|
||||
if (hit != null) return hit;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
};
|
||||
|
||||
// The root doc node is addressed as "node" (matching the mcp arg name); its
|
||||
// children are node.content[i]. The root itself is checked too so a typeless
|
||||
// root is reported rather than silently skipped.
|
||||
return walk(doc, "node");
|
||||
}
|
||||
|
||||
/**
|
||||
* Table structural node types and the container each must live directly inside.
|
||||
* Used by `insertNodeRelative` to splice rows/cells into the correct ancestor
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { findInvalidNode } from '../src/lib/node-ops.js';
|
||||
|
||||
// findInvalidNode (#409): a depth-first SHAPE gate that turns the encoder's
|
||||
// opaque `Unknown node type: undefined` into a path-anchored, pre-write
|
||||
// diagnostic. It flags the FIRST node whose `type` is absent/non-string or not
|
||||
// a known Docmost schema node, or that carries an unknown mark; returns null for
|
||||
// a well-formed doc.
|
||||
|
||||
const doc = (...content: any[]) => ({ type: 'doc', content });
|
||||
const para = (...content: any[]) => ({
|
||||
type: 'paragraph',
|
||||
attrs: { id: 'p1' },
|
||||
content,
|
||||
});
|
||||
const text = (value: string, marks?: any[]) => {
|
||||
const node: any = { type: 'text', text: value };
|
||||
if (marks) node.marks = marks;
|
||||
return node;
|
||||
};
|
||||
|
||||
describe('findInvalidNode', () => {
|
||||
it('returns null for a fully valid document', () => {
|
||||
const good = doc(
|
||||
para(text('hello ', [{ type: 'bold' }]), text('world')),
|
||||
{
|
||||
type: 'heading',
|
||||
attrs: { id: 'h1', level: 2 },
|
||||
content: [text('Title')],
|
||||
},
|
||||
);
|
||||
expect(findInvalidNode(good)).toBeNull();
|
||||
});
|
||||
|
||||
it('flags a NESTED typeless text leaf with a path-precise summary', () => {
|
||||
// A text leaf written as {"text":"foo"} with no "type":"text" — the dominant
|
||||
// `Unknown node type: undefined` cause.
|
||||
const bad = doc(
|
||||
para(text('ok')),
|
||||
para({ text: 'foo', marks: [] } as any),
|
||||
);
|
||||
const hit = findInvalidNode(bad);
|
||||
expect(hit).not.toBeNull();
|
||||
// Second paragraph (index 1), first child (index 0).
|
||||
expect(hit!.path).toBe('node.content[1].content[0]');
|
||||
expect(hit!.summary).toContain('node.content[1].content[0]');
|
||||
expect(hit!.summary).toContain('missing "type"');
|
||||
expect(hit!.summary).toContain('keys: text, marks');
|
||||
expect(hit!.summary).toContain('did you mean {"type": "text", ...}');
|
||||
});
|
||||
|
||||
it('flags a non-string type (e.g. numeric)', () => {
|
||||
const bad = doc(para({ type: 123, content: [] } as any));
|
||||
const hit = findInvalidNode(bad);
|
||||
expect(hit).not.toBeNull();
|
||||
expect(hit!.path).toBe('node.content[0].content[0]');
|
||||
expect(hit!.summary).toContain('missing "type"');
|
||||
});
|
||||
|
||||
it('flags an UNKNOWN node type name that is not in the Docmost schema', () => {
|
||||
const bad = doc({
|
||||
type: 'paragraf', // typo — not a real node
|
||||
attrs: { id: 'x' },
|
||||
content: [text('hi')],
|
||||
});
|
||||
const hit = findInvalidNode(bad);
|
||||
expect(hit).not.toBeNull();
|
||||
expect(hit!.path).toBe('node.content[0]');
|
||||
expect(hit!.summary).toContain('unknown node type "paragraf"');
|
||||
expect(hit!.summary).toContain('not in the Docmost schema');
|
||||
});
|
||||
|
||||
it('flags an UNKNOWN mark type on an otherwise-valid node', () => {
|
||||
const bad = doc(para(text('hi', [{ type: 'blink' }])));
|
||||
const hit = findInvalidNode(bad);
|
||||
expect(hit).not.toBeNull();
|
||||
expect(hit!.path).toBe('node.content[0].content[0].marks[0]');
|
||||
expect(hit!.summary).toContain('unknown mark type "blink"');
|
||||
});
|
||||
|
||||
it('accepts every real Docmost node/mark type it is asked about', () => {
|
||||
// Known node (callout) and known marks (italic, code) must NOT be flagged.
|
||||
const good = doc({
|
||||
type: 'callout',
|
||||
attrs: { id: 'c1', type: 'info' },
|
||||
content: [para(text('x', [{ type: 'italic' }, { type: 'code' }]))],
|
||||
});
|
||||
expect(findInvalidNode(good)).toBeNull();
|
||||
});
|
||||
|
||||
it('reports the root itself when the root is typeless', () => {
|
||||
const hit = findInvalidNode({ content: [] } as any);
|
||||
expect(hit).not.toBeNull();
|
||||
expect(hit!.path).toBe('node');
|
||||
});
|
||||
|
||||
it('is null-safe for non-object input', () => {
|
||||
expect(findInvalidNode(null)).toBeNull();
|
||||
expect(findInvalidNode(undefined)).toBeNull();
|
||||
expect(findInvalidNode('nope' as any)).toBeNull();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user