refactor(ai-chat): shared tool-spec registry for identical tools; formalize integration db factory
Implements two architecture follow-ups from the multi-aspect review.
1. Shared, zod-agnostic tool-spec registry (packages/mcp/src/tool-specs.ts)
for the 14 AI tools whose name + schema + model-facing description are
genuinely identical across the standalone MCP server and the in-app
AI-SDK chat. Both layers consume it (registerShared in index.ts;
sharedTool in ai-chat-tools.service.ts) and keep their own execute/auth.
- Zod-agnostic builders (z) => ZodRawShape bridge the zod v3 (mcp) vs
zod v4 (server) split; the registry imports no zod.
- Folds in the documented edit_page_text drift-bug fix: the stale
"strip-and-retry tolerated" claim is gone; canonical wording states a
formatting-only change is refused into failed[].
- Sibling-tool references in shared descriptions are transport-neutral so
one description is correct for both snake_case (MCP) and camelCase
(in-app) tool names.
- Loader fail-fast guard for a stale @docmost/mcp build.
- The ~17 intentionally-divergent tools (security guardrails, tuned UX)
stay per-layer, untouched.
- Rebuilt committed mcp artifacts (also regenerates a previously stale
build/lib/docmost-schema.js to match its already-committed source).
2. Formalize apps/server/test/integration/db.ts as the canonical
integration-test seed factory (module doc + a shortId helper); the
hand-written minimal seeders are kept on purpose, decoupled from the
app service-layer side effects.
Verified: server tsc + lint clean, mcp build clean; mcp unit tests 261 pass,
ai-chat-tools.service 16 pass, public-share-chat-tools 8 pass, ai-chat suite
224 pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,31 @@ import { CamelCasePlugin, Kysely } from 'kysely';
|
||||
import { PostgresJSDialect } from 'kysely-postgres-js';
|
||||
import * as postgres from 'postgres';
|
||||
|
||||
/**
|
||||
* db.ts — THE canonical place to seed prerequisite rows for integration tests.
|
||||
*
|
||||
* Seeders here use minimal, explicit `insertInto(...).values(...)` calls and are
|
||||
* DELIBERATELY decoupled from the app's repo `insert*` methods. Those repo
|
||||
* methods carry side effects integration specs do not want — password hashing,
|
||||
* validation, default/derived columns, event emission — so reproducing only the
|
||||
* columns a test needs keeps the fixtures small, fast and predictable.
|
||||
*
|
||||
* CONVENTIONS:
|
||||
* - New entity seeders go HERE (a `createX(db, ...)` helper) rather than as raw
|
||||
* `insertInto` calls scattered across spec files, so the schema knowledge
|
||||
* lives in one place.
|
||||
* - Each seeder inserts only the NOT NULL / uniquely-constrained columns plus
|
||||
* whatever the consuming tests assert on; everything else is left to DB
|
||||
* defaults.
|
||||
* - Plain `randomUUID()` (v4) is fine for FK integrity; the app uses uuid v7,
|
||||
* but tests never depend on id ordering.
|
||||
*
|
||||
* TRADE-OFF: because the column/constraint knowledge below is mirrored from the
|
||||
* Kysely schema rather than derived from it, a migration that changes a NOT NULL
|
||||
* column or a unique constraint can make an insert here fail. When that happens
|
||||
* the fix is to update the relevant seeder, not the spec that calls it.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Isolated test database connection string. The dev DB is `docmost`; tests run
|
||||
* against a dedicated `docmost_test` that global-setup drops + recreates +
|
||||
@@ -58,21 +83,27 @@ export async function destroyTestDb(): Promise<void> {
|
||||
}
|
||||
|
||||
// --- Seeding helpers ---------------------------------------------------------
|
||||
// Insert minimal valid rows (only the columns the tests need + NOT NULL ones).
|
||||
// Plain randomUUID() is fine for FK integrity in tests (the app uses uuid v7).
|
||||
// Each helper inserts a minimal valid row (only the columns the tests need plus
|
||||
// the NOT NULL / uniquely-constrained ones) and returns the generated id. See
|
||||
// the module doc comment above for why these bypass the app's repo layer.
|
||||
|
||||
// Short, human-readable suffix derived from a row's uuid. Used to build unique
|
||||
// names/slugs/hostnames for seeded rows so unique constraints never collide.
|
||||
const shortId = (id: string): string => id.slice(0, 8);
|
||||
|
||||
export async function createWorkspace(
|
||||
db: Kysely<any>,
|
||||
overrides: { settings?: unknown; name?: string } = {},
|
||||
): Promise<{ id: string; settings: any }> {
|
||||
const id = randomUUID();
|
||||
const suffix = shortId(id);
|
||||
const row = await db
|
||||
.insertInto('workspaces')
|
||||
.values({
|
||||
id,
|
||||
name: overrides.name ?? `ws-${id.slice(0, 8)}`,
|
||||
name: overrides.name ?? `ws-${suffix}`,
|
||||
// hostname is uniquely constrained; keep it unique per workspace.
|
||||
hostname: `host-${id.slice(0, 8)}`,
|
||||
hostname: `host-${suffix}`,
|
||||
settings: overrides.settings === undefined ? null : (overrides.settings as any),
|
||||
})
|
||||
.returning(['id', 'settings'])
|
||||
@@ -86,12 +117,13 @@ export async function createUser(
|
||||
overrides: { email?: string; name?: string } = {},
|
||||
): Promise<{ id: string }> {
|
||||
const id = randomUUID();
|
||||
const suffix = shortId(id);
|
||||
const row = await db
|
||||
.insertInto('users')
|
||||
.values({
|
||||
id,
|
||||
email: overrides.email ?? `user-${id.slice(0, 8)}@example.test`,
|
||||
name: overrides.name ?? `user-${id.slice(0, 8)}`,
|
||||
email: overrides.email ?? `user-${suffix}@example.test`,
|
||||
name: overrides.name ?? `user-${suffix}`,
|
||||
workspaceId,
|
||||
})
|
||||
.returning(['id'])
|
||||
@@ -105,13 +137,14 @@ export async function createSpace(
|
||||
overrides: { slug?: string; name?: string } = {},
|
||||
): Promise<{ id: string }> {
|
||||
const id = randomUUID();
|
||||
const suffix = shortId(id);
|
||||
const row = await db
|
||||
.insertInto('spaces')
|
||||
.values({
|
||||
id,
|
||||
name: overrides.name ?? `space-${id.slice(0, 8)}`,
|
||||
name: overrides.name ?? `space-${suffix}`,
|
||||
// slug is unique per workspace + NOT NULL.
|
||||
slug: overrides.slug ?? `space-${id.slice(0, 8)}`,
|
||||
slug: overrides.slug ?? `space-${suffix}`,
|
||||
workspaceId,
|
||||
})
|
||||
.returning(['id'])
|
||||
@@ -124,13 +157,14 @@ export async function createPage(
|
||||
args: { workspaceId: string; spaceId: string; title?: string },
|
||||
): Promise<{ id: string }> {
|
||||
const id = randomUUID();
|
||||
const suffix = shortId(id);
|
||||
const row = await db
|
||||
.insertInto('pages')
|
||||
.values({
|
||||
id,
|
||||
// slug_id is NOT NULL + globally unique.
|
||||
slugId: `slug-${id.slice(0, 8)}`,
|
||||
title: args.title ?? `page-${id.slice(0, 8)}`,
|
||||
slugId: `slug-${suffix}`,
|
||||
title: args.title ?? `page-${suffix}`,
|
||||
spaceId: args.spaceId,
|
||||
workspaceId: args.workspaceId,
|
||||
})
|
||||
@@ -186,7 +220,7 @@ export async function createChat(
|
||||
workspaceId: args.workspaceId,
|
||||
creatorId: args.creatorId,
|
||||
roleId: args.roleId ?? null,
|
||||
title: args.title ?? `chat-${id.slice(0, 8)}`,
|
||||
title: args.title ?? `chat-${shortId(id)}`,
|
||||
})
|
||||
.returning(['id'])
|
||||
.executeTakeFirstOrThrow();
|
||||
|
||||
Reference in New Issue
Block a user