From 00eee179ee3f135a23e989825346260a0a6ea2ff Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Sun, 30 Aug 2026 18:34:22 +0300 Subject: [PATCH 1/4] feat: add workspace catalog schema and catalog-backed reads (#113 Phase A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces the durable SQLite catalog from docs/specifications/workspace-sharding.md (spaces, catalog_documents, catalog_collections, record_locator, catalog_revisions, catalog_outbox) as the source of truth for Document/Collection titles and hierarchy, kept in sync by dual-writing from the service layer alongside each Y.Doc mutation. The existing single global Y.Doc stays the one real content shard for now — this is the first of several phased slices toward #113's full shard-aware architecture, not the full cutover (see docs/specifications/workspace-sharding.md's approved design). - New tables + ensureCatalogBootstrapped() dev/test backfill (src/lib/server/catalog.ts) - documents.ts/collections.ts dual-write on create/rename/move/delete, replacing silent id-collision overwrite with RecordIdConflictError (§3.1's locator reservation) - +page.server.ts/+layout.server.ts now read the catalog instead of the Y.Doc directly - parentDocumentId is deliberately not a hard FK: a Document can be created by a client writing directly to the Y.Doc over Yjs sync, bypassing the service layer entirely (a supported pattern per audit-coverage.md) — verified via tier-a.test.ts Refs #113 Co-Authored-By: Claude Sonnet 5 --- drizzle/0002_salty_kid_colt.sql | 58 +++ drizzle/meta/0002_snapshot.json | 549 +++++++++++++++++++++++++ drizzle/meta/_journal.json | 7 + src/lib/server/catalog.test.ts | 234 +++++++++++ src/lib/server/catalog.ts | 371 +++++++++++++++++ src/lib/server/db/index.ts | 5 + src/lib/server/db/schema.ts | 101 ++++- src/lib/server/workspace-store.test.ts | 10 + src/lib/server/workspace-store.ts | 11 +- src/lib/services/collections.ts | 28 +- src/lib/services/documents.ts | 41 +- src/lib/services/services.test.ts | 62 +++ src/routes/+layout.server.ts | 9 +- src/routes/+page.server.ts | 9 +- 14 files changed, 1476 insertions(+), 19 deletions(-) create mode 100644 drizzle/0002_salty_kid_colt.sql create mode 100644 drizzle/meta/0002_snapshot.json create mode 100644 src/lib/server/catalog.test.ts create mode 100644 src/lib/server/catalog.ts diff --git a/drizzle/0002_salty_kid_colt.sql b/drizzle/0002_salty_kid_colt.sql new file mode 100644 index 0000000..a2d8dc3 --- /dev/null +++ b/drizzle/0002_salty_kid_colt.sql @@ -0,0 +1,58 @@ +CREATE TABLE `catalog_collections` ( + `id` text PRIMARY KEY NOT NULL, + `workspace_id` text DEFAULT 'default' NOT NULL, + `space_id` text NOT NULL, + `shard_id` text DEFAULT 'default' NOT NULL, + `title` text NOT NULL, + `created_at` integer NOT NULL, + `updated_at` integer NOT NULL, + FOREIGN KEY (`space_id`) REFERENCES `spaces`(`id`) ON UPDATE no action ON DELETE no action +); +--> statement-breakpoint +CREATE TABLE `catalog_documents` ( + `id` text PRIMARY KEY NOT NULL, + `workspace_id` text DEFAULT 'default' NOT NULL, + `space_id` text NOT NULL, + `shard_id` text DEFAULT 'default' NOT NULL, + `title` text NOT NULL, + `parent_document_id` text, + `order` text NOT NULL, + `created_at` integer NOT NULL, + `updated_at` integer NOT NULL, + FOREIGN KEY (`space_id`) REFERENCES `spaces`(`id`) ON UPDATE no action ON DELETE no action +); +--> statement-breakpoint +CREATE TABLE `catalog_outbox` ( + `id` integer PRIMARY KEY AUTOINCREMENT NOT NULL, + `workspace_id` text DEFAULT 'default' NOT NULL, + `revision` integer NOT NULL, + `status` text NOT NULL, + `operation_id` text NOT NULL, + `payload_json` text NOT NULL, + `created_at` integer NOT NULL, + `published_at` integer +); +--> statement-breakpoint +CREATE TABLE `catalog_revisions` ( + `workspace_id` text PRIMARY KEY DEFAULT 'default' NOT NULL, + `revision` integer DEFAULT 0 NOT NULL +); +--> statement-breakpoint +CREATE TABLE `record_locator` ( + `id` integer PRIMARY KEY AUTOINCREMENT NOT NULL, + `workspace_id` text DEFAULT 'default' NOT NULL, + `record_id` text NOT NULL, + `kind` text NOT NULL, + `space_id` text NOT NULL, + `shard_id` text DEFAULT 'default' NOT NULL, + `created_at` integer NOT NULL, + FOREIGN KEY (`space_id`) REFERENCES `spaces`(`id`) ON UPDATE no action ON DELETE no action +); +--> statement-breakpoint +CREATE UNIQUE INDEX `record_locator_workspace_record_unique` ON `record_locator` (`workspace_id`,`record_id`);--> statement-breakpoint +CREATE TABLE `spaces` ( + `id` text PRIMARY KEY NOT NULL, + `workspace_id` text DEFAULT 'default' NOT NULL, + `name` text NOT NULL, + `created_at` integer NOT NULL +); diff --git a/drizzle/meta/0002_snapshot.json b/drizzle/meta/0002_snapshot.json new file mode 100644 index 0000000..0ebe15e --- /dev/null +++ b/drizzle/meta/0002_snapshot.json @@ -0,0 +1,549 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "7b84718e-e792-4956-a333-a01efe8f0485", + "prevId": "ae36812a-00d1-455c-acae-ba1fde05dbb1", + "tables": { + "access_tokens": { + "name": "access_tokens", + "columns": { + "token_hash": { + "name": "token_hash", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "client_label": { + "name": "client_label", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "allowed_document_ids": { + "name": "allowed_document_ids", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "allowed_collection_ids": { + "name": "allowed_collection_ids", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "revoked_at": { + "name": "revoked_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "audit_log": { + "name": "audit_log", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "actor_json": { + "name": "actor_json", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "action": { + "name": "action", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "target_record_id": { + "name": "target_record_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "timestamp": { + "name": "timestamp", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "diff_json": { + "name": "diff_json", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "catalog_collections": { + "name": "catalog_collections", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "workspace_id": { + "name": "workspace_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "space_id": { + "name": "space_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "shard_id": { + "name": "shard_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "catalog_collections_space_id_spaces_id_fk": { + "name": "catalog_collections_space_id_spaces_id_fk", + "tableFrom": "catalog_collections", + "tableTo": "spaces", + "columnsFrom": [ + "space_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "catalog_documents": { + "name": "catalog_documents", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "workspace_id": { + "name": "workspace_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "space_id": { + "name": "space_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "shard_id": { + "name": "shard_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "parent_document_id": { + "name": "parent_document_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "order": { + "name": "order", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "catalog_documents_space_id_spaces_id_fk": { + "name": "catalog_documents_space_id_spaces_id_fk", + "tableFrom": "catalog_documents", + "tableTo": "spaces", + "columnsFrom": [ + "space_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "catalog_outbox": { + "name": "catalog_outbox", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "workspace_id": { + "name": "workspace_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "revision": { + "name": "revision", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "operation_id": { + "name": "operation_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "payload_json": { + "name": "payload_json", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "published_at": { + "name": "published_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "catalog_revisions": { + "name": "catalog_revisions", + "columns": { + "workspace_id": { + "name": "workspace_id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "revision": { + "name": "revision", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "record_locator": { + "name": "record_locator", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "workspace_id": { + "name": "workspace_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "record_id": { + "name": "record_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "kind": { + "name": "kind", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "space_id": { + "name": "space_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "shard_id": { + "name": "shard_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "record_locator_workspace_record_unique": { + "name": "record_locator_workspace_record_unique", + "columns": [ + "workspace_id", + "record_id" + ], + "isUnique": true + } + }, + "foreignKeys": { + "record_locator_space_id_spaces_id_fk": { + "name": "record_locator_space_id_spaces_id_fk", + "tableFrom": "record_locator", + "tableTo": "spaces", + "columnsFrom": [ + "space_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "snapshots": { + "name": "snapshots", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "workspace_id": { + "name": "workspace_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "shard_id": { + "name": "shard_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "state": { + "name": "state", + "type": "blob", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "spaces": { + "name": "spaces", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "workspace_id": { + "name": "workspace_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'default'" + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} \ No newline at end of file diff --git a/drizzle/meta/_journal.json b/drizzle/meta/_journal.json index 48e4ded..d030f03 100644 --- a/drizzle/meta/_journal.json +++ b/drizzle/meta/_journal.json @@ -15,6 +15,13 @@ "when": 1787770170816, "tag": "0001_chilly_tyger_tiger", "breakpoints": true + }, + { + "idx": 2, + "version": "6", + "when": 1788103543215, + "tag": "0002_salty_kid_colt", + "breakpoints": true } ] } \ No newline at end of file diff --git a/src/lib/server/catalog.test.ts b/src/lib/server/catalog.test.ts new file mode 100644 index 0000000..e9fd2d7 --- /dev/null +++ b/src/lib/server/catalog.test.ts @@ -0,0 +1,234 @@ +import { describe, expect, it } from 'vitest'; +import * as Y from 'yjs'; +import { eq } from 'drizzle-orm'; +import { getDb } from './store'; +import { catalogDocuments, catalogOutbox, catalogRevisions, spaces } from './db/schema'; +import { + createDocument as crdtCreateDocument, + createCollection as crdtCreateCollection +} from '$lib/data/records'; +import { + ensureCatalogBootstrapped, + listCatalogCollections, + listCatalogDocuments, + recordCatalogDocumentCreated, + recordCatalogDocumentDeleted, + recordCatalogDocumentMoved, + recordCatalogDocumentTitleChanged, + RecordIdConflictError, + reserveDocumentLocator +} from './catalog'; + +const WS = 'default'; +const SHARD = 'default'; + +function bootstrap() { + const doc = new Y.Doc(); + return { doc, ...ensureCatalogBootstrapped(WS, SHARD, doc) }; +} + +describe('catalog: record locator uniqueness (#113 Phase A, §3.1)', () => { + it('rejects a duplicate (workspaceId, recordId) reservation', () => { + const { defaultSpaceId } = bootstrap(); + reserveDocumentLocator(WS, defaultSpaceId, 'doc-1', SHARD); + expect(() => reserveDocumentLocator(WS, defaultSpaceId, 'doc-1', SHARD)).toThrow( + RecordIdConflictError + ); + }); +}); + +describe('catalog: bootstrap and backfill', () => { + it('creates exactly one default Space, idempotently, even across repeated calls', () => { + const doc = new Y.Doc(); + const first = ensureCatalogBootstrapped(WS, SHARD, doc); + const second = ensureCatalogBootstrapped(WS, SHARD, doc); + expect(second.defaultSpaceId).toBe(first.defaultSpaceId); + + const rows = getDb().select().from(spaces).where(eq(spaces.workspaceId, WS)).all(); + expect(rows).toHaveLength(1); + }); + + it('backfills existing Y.Doc documents and collections into the catalog on first bootstrap', () => { + const doc = new Y.Doc(); + const existingDoc = crdtCreateDocument(doc, { title: 'Pre-existing Doc' }); + const existingCollection = crdtCreateCollection(doc, { + title: 'Pre-existing Table', + schema: [] + }); + + ensureCatalogBootstrapped(WS, SHARD, doc); + + const docs = listCatalogDocuments(WS); + const collections = listCatalogCollections(WS); + expect(docs.find((d) => d.id === existingDoc.id)?.title).toBe('Pre-existing Doc'); + expect(collections.find((c) => c.id === existingCollection.id)?.title).toBe( + 'Pre-existing Table' + ); + }); +}); + +describe('catalog: committed writes bump revision and append a published outbox row', () => { + it('increments the workspace revision and records an outbox entry on document create', () => { + const { defaultSpaceId } = bootstrap(); + const before = + getDb() + .select({ revision: catalogRevisions.revision }) + .from(catalogRevisions) + .where(eq(catalogRevisions.workspaceId, WS)) + .get()?.revision ?? 0; + + reserveDocumentLocator(WS, defaultSpaceId, 'doc-rev', SHARD); + recordCatalogDocumentCreated({ + workspaceId: WS, + spaceId: defaultSpaceId, + id: 'doc-rev', + title: 'Revision Doc', + order: 'a0', + shardId: SHARD + }); + + const after = getDb() + .select({ revision: catalogRevisions.revision }) + .from(catalogRevisions) + .where(eq(catalogRevisions.workspaceId, WS)) + .get()?.revision; + expect(after).toBe(before + 1); + + const outboxRows = getDb() + .select() + .from(catalogOutbox) + .where(eq(catalogOutbox.workspaceId, WS)) + .all(); + const created = outboxRows.at(-1); + expect(created?.status).toBe('published'); + expect(created?.revision).toBe(after); + expect(created?.payload).toMatchObject({ documents: ['doc-rev'], op: 'create' }); + }); +}); + +describe('catalog: parentDocumentId tolerates a parent not yet in the catalog', () => { + it('creates a nested document even when its parent was never cataloged (e.g. written directly to the Y.Doc over Yjs sync, bypassing the service layer)', () => { + const { defaultSpaceId } = bootstrap(); + // Simulates a Document created by a direct Yjs client write, never + // going through reserveDocumentLocator/recordCatalogDocumentCreated — + // a supported pattern (see db/schema.ts's parentDocumentId comment). + const uncatalogedParentId = 'uncataloged-parent'; + + reserveDocumentLocator(WS, defaultSpaceId, 'nested-child', SHARD); + expect(() => + recordCatalogDocumentCreated({ + workspaceId: WS, + spaceId: defaultSpaceId, + id: 'nested-child', + title: 'Nested Child', + parentDocumentId: uncatalogedParentId, + order: 'a0', + shardId: SHARD + }) + ).not.toThrow(); + + const child = listCatalogDocuments(WS).find((d) => d.id === 'nested-child'); + expect(child?.parentDocumentId).toBe(uncatalogedParentId); + + // Deleting the (never-cataloged) "parent" is a safe no-op, not a crash — + // it never had a catalog row to begin with. + expect(() => recordCatalogDocumentDeleted(WS, uncatalogedParentId)).not.toThrow(); + expect(listCatalogDocuments(WS).find((d) => d.id === 'nested-child')).toBeDefined(); + }); +}); + +describe('catalog: document deletion cascades to descendants', () => { + it('deletes a document and its nested children from the catalog in one call', () => { + const { defaultSpaceId } = bootstrap(); + + reserveDocumentLocator(WS, defaultSpaceId, 'parent', SHARD); + recordCatalogDocumentCreated({ + workspaceId: WS, + spaceId: defaultSpaceId, + id: 'parent', + title: 'Parent', + order: 'a0', + shardId: SHARD + }); + reserveDocumentLocator(WS, defaultSpaceId, 'child', SHARD); + recordCatalogDocumentCreated({ + workspaceId: WS, + spaceId: defaultSpaceId, + id: 'child', + title: 'Child', + parentDocumentId: 'parent', + order: 'a0', + shardId: SHARD + }); + + recordCatalogDocumentDeleted(WS, 'parent'); + + const remaining = getDb() + .select() + .from(catalogDocuments) + .where(eq(catalogDocuments.workspaceId, WS)) + .all(); + expect(remaining.find((d) => d.id === 'parent')).toBeUndefined(); + expect(remaining.find((d) => d.id === 'child')).toBeUndefined(); + + // The freed id must be reservable again — proves the record_locator rows + // for both parent and child were actually cleaned up, not just the + // catalog_documents rows via FK cascade. + expect(() => reserveDocumentLocator(WS, defaultSpaceId, 'parent', SHARD)).not.toThrow(); + expect(() => reserveDocumentLocator(WS, defaultSpaceId, 'child', SHARD)).not.toThrow(); + }); + + it('moving a document updates its catalog parent/order', () => { + const { defaultSpaceId } = bootstrap(); + reserveDocumentLocator(WS, defaultSpaceId, 'movable', SHARD); + recordCatalogDocumentCreated({ + workspaceId: WS, + spaceId: defaultSpaceId, + id: 'movable', + title: 'Movable', + order: 'a0', + shardId: SHARD + }); + reserveDocumentLocator(WS, defaultSpaceId, 'new-parent', SHARD); + recordCatalogDocumentCreated({ + workspaceId: WS, + spaceId: defaultSpaceId, + id: 'new-parent', + title: 'New Parent', + order: 'a1', + shardId: SHARD + }); + + recordCatalogDocumentMoved(WS, 'movable', 'new-parent', 'a2'); + + const row = getDb() + .select() + .from(catalogDocuments) + .where(eq(catalogDocuments.id, 'movable')) + .get(); + expect(row?.parentDocumentId).toBe('new-parent'); + expect(row?.order).toBe('a2'); + }); + + it('renaming a document updates its catalog title', () => { + const { defaultSpaceId } = bootstrap(); + reserveDocumentLocator(WS, defaultSpaceId, 'renamable', SHARD); + recordCatalogDocumentCreated({ + workspaceId: WS, + spaceId: defaultSpaceId, + id: 'renamable', + title: 'Before', + order: 'a0', + shardId: SHARD + }); + + recordCatalogDocumentTitleChanged(WS, 'renamable', 'After'); + + const row = getDb() + .select() + .from(catalogDocuments) + .where(eq(catalogDocuments.id, 'renamable')) + .get(); + expect(row?.title).toBe('After'); + }); +}); diff --git a/src/lib/server/catalog.ts b/src/lib/server/catalog.ts new file mode 100644 index 0000000..efcb69a --- /dev/null +++ b/src/lib/server/catalog.ts @@ -0,0 +1,371 @@ +import * as Y from 'yjs'; +import { and, eq, inArray, sql } from 'drizzle-orm'; +import { nanoid } from 'nanoid'; +import { getDb } from './store.js'; +import { + catalogCollections, + catalogDocuments, + catalogOutbox, + catalogRevisions, + recordLocator, + spaces +} from './db/schema.js'; +import { + listCollections as crdtListCollections, + listDocuments as crdtListDocuments +} from '../data/records.js'; +import type { CollectionMeta, DocumentMeta, ParentKind } from '../data/types.js'; + +// The catalog: durable SQLite metadata fronting the (today: single, Phase B: +// per-Document/per-Collection) Y.Doc content shard(s) — see +// docs/specifications/workspace-sharding.md §3.1. Phase A dual-writes this +// alongside every service-layer Document/Collection mutation, without yet +// splitting the Y.Doc itself (every row's shardId stays 'default'). Placed +// under src/lib/server/, not src/lib/services/, so it is never picked up by +// src/lib/services/manifest.ts's MCP tool-surface registration. + +export class RecordIdConflictError extends Error { + constructor(recordId: string) { + super(`Record id ${recordId} already exists in this workspace`); + this.name = 'RecordIdConflictError'; + } +} + +type CatalogOp = 'create' | 'update' | 'move' | 'delete'; + +function bumpRevisionAndAppendOutbox( + workspaceId: string, + payload: { documents?: string[]; collections?: string[]; op: CatalogOp } +): void { + const db = getDb(); + db.insert(catalogRevisions) + .values({ workspaceId, revision: 1 }) + .onConflictDoUpdate({ + target: catalogRevisions.workspaceId, + set: { revision: sql`${catalogRevisions.revision} + 1` } + }) + .run(); + const revision = + db + .select({ revision: catalogRevisions.revision }) + .from(catalogRevisions) + .where(eq(catalogRevisions.workspaceId, workspaceId)) + .get()?.revision ?? 1; + const now = Date.now(); + db.insert(catalogOutbox) + .values({ + workspaceId, + revision, + status: 'published', + operationId: nanoid(), + payload, + createdAt: now, + publishedAt: now + }) + .run(); +} + +/** + * Reserves `id` in the workspace-wide record locator before any content is + * written for it — the mechanism behind §3.1's "a duplicate + * (workspace_id, record_id) is rejected." Throws RecordIdConflictError + * *before* the Y.Doc is touched on a collision, replacing the prior + * silent-overwrite-on-duplicate-id behavior of data/records.ts's + * createDocument/createCollection. + */ +function reserveLocator( + workspaceId: string, + spaceId: string, + recordId: string, + kind: ParentKind, + shardId: string +): void { + try { + getDb() + .insert(recordLocator) + .values({ workspaceId, recordId, kind, spaceId, shardId, createdAt: Date.now() }) + .run(); + } catch (err) { + if (err instanceof Error && /UNIQUE constraint failed/.test(err.message)) { + throw new RecordIdConflictError(recordId); + } + throw err; + } +} + +export function reserveDocumentLocator( + workspaceId: string, + spaceId: string, + id: string, + shardId: string +): void { + reserveLocator(workspaceId, spaceId, id, 'document', shardId); +} + +export function reserveCollectionLocator( + workspaceId: string, + spaceId: string, + id: string, + shardId: string +): void { + reserveLocator(workspaceId, spaceId, id, 'collection', shardId); +} + +export function recordCatalogDocumentCreated(input: { + workspaceId: string; + spaceId: string; + id: string; + title: string; + parentDocumentId?: string; + order: string; + shardId: string; +}): void { + const db = getDb(); + const now = Date.now(); + db.transaction((tx) => { + tx.insert(catalogDocuments) + .values({ + id: input.id, + workspaceId: input.workspaceId, + spaceId: input.spaceId, + shardId: input.shardId, + title: input.title, + parentDocumentId: input.parentDocumentId, + order: input.order, + createdAt: now, + updatedAt: now + }) + .run(); + }); + bumpRevisionAndAppendOutbox(input.workspaceId, { documents: [input.id], op: 'create' }); +} + +export function recordCatalogDocumentTitleChanged( + workspaceId: string, + id: string, + title: string +): void { + getDb() + .update(catalogDocuments) + .set({ title, updatedAt: Date.now() }) + .where(eq(catalogDocuments.id, id)) + .run(); + bumpRevisionAndAppendOutbox(workspaceId, { documents: [id], op: 'update' }); +} + +export function recordCatalogDocumentMoved( + workspaceId: string, + id: string, + parentDocumentId: string | undefined, + order: string +): void { + getDb() + .update(catalogDocuments) + .set({ parentDocumentId: parentDocumentId ?? null, order, updatedAt: Date.now() }) + .where(eq(catalogDocuments.id, id)) + .run(); + bumpRevisionAndAppendOutbox(workspaceId, { documents: [id], op: 'move' }); +} + +/** + * Deletes a Document and its descendants from the catalog, mirroring + * data/records.ts's recursive deleteDocument. Walks the *catalog's own* + * parent chain (via a recursive CTE — drizzle's typed query builder has no + * WITH RECURSIVE support) rather than re-deriving it from the Y.Doc; these + * should always agree since every prior create/move immediately mirrors into + * the catalog, but a prior undetected divergence would under-cascade here. + * A no-op (not an error) if `id` has no catalog row at all — e.g. it was + * created by a client writing directly to the Y.Doc, bypassing the service + * layer (see parentDocumentId's comment in db/schema.ts). + */ +export function recordCatalogDocumentDeleted(workspaceId: string, id: string): void { + const db = getDb(); + const descendants = db.all<{ id: string }>(sql` + WITH RECURSIVE descendants(id) AS ( + SELECT id FROM catalog_documents WHERE id = ${id} AND workspace_id = ${workspaceId} + UNION ALL + SELECT catalog_documents.id FROM catalog_documents + JOIN descendants ON catalog_documents.parent_document_id = descendants.id + WHERE catalog_documents.workspace_id = ${workspaceId} + ) + SELECT id FROM descendants + `); + const ids = descendants.map((row) => row.id); + if (ids.length === 0) return; + + db.transaction((tx) => { + for (const docId of ids) { + tx.delete(recordLocator) + .where(and(eq(recordLocator.workspaceId, workspaceId), eq(recordLocator.recordId, docId))) + .run(); + } + // parentDocumentId isn't a real FK (see db/schema.ts), so every + // descendant is deleted explicitly rather than relying on a cascade. + tx.delete(catalogDocuments).where(inArray(catalogDocuments.id, ids)).run(); + }); + bumpRevisionAndAppendOutbox(workspaceId, { documents: ids, op: 'delete' }); +} + +export function recordCatalogCollectionCreated(input: { + workspaceId: string; + spaceId: string; + id: string; + title: string; + shardId: string; +}): void { + const db = getDb(); + const now = Date.now(); + db.transaction((tx) => { + tx.insert(catalogCollections) + .values({ + id: input.id, + workspaceId: input.workspaceId, + spaceId: input.spaceId, + shardId: input.shardId, + title: input.title, + createdAt: now, + updatedAt: now + }) + .run(); + }); + bumpRevisionAndAppendOutbox(input.workspaceId, { collections: [input.id], op: 'create' }); +} + +export function recordCatalogCollectionTitleChanged( + workspaceId: string, + id: string, + title: string +): void { + getDb() + .update(catalogCollections) + .set({ title, updatedAt: Date.now() }) + .where(eq(catalogCollections.id, id)) + .run(); + bumpRevisionAndAppendOutbox(workspaceId, { collections: [id], op: 'update' }); +} + +export function recordCatalogCollectionDeleted(workspaceId: string, id: string): void { + const db = getDb(); + db.transaction((tx) => { + tx.delete(recordLocator) + .where(and(eq(recordLocator.workspaceId, workspaceId), eq(recordLocator.recordId, id))) + .run(); + tx.delete(catalogCollections).where(eq(catalogCollections.id, id)).run(); + }); + bumpRevisionAndAppendOutbox(workspaceId, { collections: [id], op: 'delete' }); +} + +export function listCatalogDocuments(workspaceId: string): DocumentMeta[] { + return getDb() + .select() + .from(catalogDocuments) + .where(eq(catalogDocuments.workspaceId, workspaceId)) + .all() + .map((row) => ({ + id: row.id, + title: row.title, + parentDocumentId: row.parentDocumentId ?? undefined, + order: row.order, + recordIds: [] + })) + .sort((a, b) => a.order.localeCompare(b.order)); +} + +export function listCatalogCollections(workspaceId: string): CollectionMeta[] { + return getDb() + .select() + .from(catalogCollections) + .where(eq(catalogCollections.workspaceId, workspaceId)) + .all() + .map((row) => ({ + id: row.id, + title: row.title, + schema: [], + recordIds: [] + })); +} + +/** + * Ensures the catalog has a default Space for this workspace, backfilling it + * from the Y.Doc's current Documents/Collections the first time this + * resolves (idempotent via a durable-state check, not an in-memory flag — + * safe even if called more than once). This is a narrow dev/test + * convenience for Phase A's existing local data, NOT §7's versioned, + * checksum-verified production migration (that is #114's job). + */ +export function ensureCatalogBootstrapped( + workspaceId: string, + shardId: string, + doc: Y.Doc +): { defaultSpaceId: string } { + const db = getDb(); + const existing = db + .select({ id: spaces.id }) + .from(spaces) + .where(eq(spaces.workspaceId, workspaceId)) + .get(); + if (existing) return { defaultSpaceId: existing.id }; + + const defaultSpaceId = nanoid(); + const now = Date.now(); + const existingDocs = crdtListDocuments(doc); + const existingCollections = crdtListCollections(doc); + + db.transaction((tx) => { + tx.insert(spaces) + .values({ id: defaultSpaceId, workspaceId, name: 'Default', createdAt: now }) + .run(); + tx.insert(catalogRevisions).values({ workspaceId, revision: 0 }).run(); + + for (const d of existingDocs) { + tx.insert(recordLocator) + .values({ + workspaceId, + recordId: d.id, + kind: 'document', + spaceId: defaultSpaceId, + shardId, + createdAt: now + }) + .run(); + tx.insert(catalogDocuments) + .values({ + id: d.id, + workspaceId, + spaceId: defaultSpaceId, + shardId, + title: d.title, + parentDocumentId: d.parentDocumentId, + order: d.order, + createdAt: now, + updatedAt: now + }) + .run(); + } + for (const c of existingCollections) { + tx.insert(recordLocator) + .values({ + workspaceId, + recordId: c.id, + kind: 'collection', + spaceId: defaultSpaceId, + shardId, + createdAt: now + }) + .run(); + tx.insert(catalogCollections) + .values({ + id: c.id, + workspaceId, + spaceId: defaultSpaceId, + shardId, + title: c.title, + createdAt: now, + updatedAt: now + }) + .run(); + } + }); + + return { defaultSpaceId }; +} diff --git a/src/lib/server/db/index.ts b/src/lib/server/db/index.ts index db78f20..d6cd853 100644 --- a/src/lib/server/db/index.ts +++ b/src/lib/server/db/index.ts @@ -32,6 +32,11 @@ export function getDb(): Db { const client = new Database(url); client.pragma('journal_mode = WAL'); + // better-sqlite3 doesn't enforce foreign keys unless this is set per + // connection — without it, the catalog's spaceId foreign keys (see + // db/schema.ts) would silently allow an orphaned Space reference instead + // of rejecting it. + client.pragma('foreign_keys = ON'); const db = drizzle(client, { schema }); migrate(db, { migrationsFolder: 'drizzle' }); diff --git a/src/lib/server/db/schema.ts b/src/lib/server/db/schema.ts index 7b81712..a998705 100644 --- a/src/lib/server/db/schema.ts +++ b/src/lib/server/db/schema.ts @@ -1,4 +1,4 @@ -import { blob, integer, sqliteTable, text } from 'drizzle-orm/sqlite-core'; +import { blob, integer, sqliteTable, text, uniqueIndex } from 'drizzle-orm/sqlite-core'; import type { ActorId } from '$lib/data/types'; export const snapshots = sqliteTable('snapshots', { @@ -32,3 +32,102 @@ export const accessTokens = sqliteTable('access_tokens', { createdAt: integer('created_at').notNull(), revokedAt: integer('revoked_at') }); + +// --- Workspace catalog (docs/specifications/workspace-sharding.md, #113 Phase A) --- +// +// The catalog is the durable, server-owned source of truth for Space/Document/ +// Collection identity, title, and hierarchy — kept in sync by dual-writing from +// the service layer alongside each Y.Doc mutation (see src/lib/server/catalog.ts). +// Phase A keeps every row's shardId at 'default' (the one real content shard +// today); the column exists now so Phase B's real per-Document/per-Collection +// shard split is a query-scoping change, not another migration. + +export const spaces = sqliteTable('spaces', { + id: text('id').primaryKey(), + workspaceId: text('workspace_id').notNull().default('default'), + name: text('name').notNull(), + createdAt: integer('created_at').notNull() +}); + +export const catalogDocuments = sqliteTable('catalog_documents', { + id: text('id').primaryKey(), // == the Y.Doc DocumentMeta.id it mirrors + workspaceId: text('workspace_id').notNull().default('default'), + spaceId: text('space_id') + .notNull() + .references(() => spaces.id), + shardId: text('shard_id').notNull().default('default'), + title: text('title').notNull(), + // Deliberately NOT a foreign key: a Document can be created by a client + // writing directly to the Y.Doc over Yjs sync, bypassing the service layer + // entirely (a supported pattern — see docs/specifications/audit-coverage.md + // and tests/e2e/tier-a.test.ts's direct-Yjs-client cases). Its catalog row + // wouldn't exist yet, so a strict FK on a real parentDocumentId would throw + // on an otherwise-valid nested create. recordCatalogDocumentDeleted (see + // catalog.ts) therefore deletes descendants explicitly rather than relying + // on ON DELETE CASCADE. + parentDocumentId: text('parent_document_id'), + order: text('order').notNull(), // mirrors DocumentMeta.order exactly, never independently recomputed + createdAt: integer('created_at').notNull(), + updatedAt: integer('updated_at').notNull() +}); + +export const catalogCollections = sqliteTable('catalog_collections', { + id: text('id').primaryKey(), // == the Y.Doc CollectionMeta.id it mirrors + workspaceId: text('workspace_id').notNull().default('default'), + spaceId: text('space_id') + .notNull() + .references(() => spaces.id), + shardId: text('shard_id').notNull().default('default'), + title: text('title').notNull(), + // No parent/order (Collections are flat) and no schema mirror — schema + // stays shard-owned per workspace-sharding.md §3.1/§3.2. + createdAt: integer('created_at').notNull(), + updatedAt: integer('updated_at').notNull() +}); + +// The workspace-wide (workspace_id, record_id) locator required by §3.1: the +// mechanism that actually rejects a duplicate id across Documents/Collections +// (today's separate Y.Maps for each don't prevent that at all). +export const recordLocator = sqliteTable( + 'record_locator', + { + id: integer('id').primaryKey({ autoIncrement: true }), + workspaceId: text('workspace_id').notNull().default('default'), + recordId: text('record_id').notNull(), + kind: text('kind').notNull().$type<'document' | 'collection'>(), + spaceId: text('space_id') + .notNull() + .references(() => spaces.id), + shardId: text('shard_id').notNull().default('default'), + createdAt: integer('created_at').notNull() + }, + (t) => [uniqueIndex('record_locator_workspace_record_unique').on(t.workspaceId, t.recordId)] +); + +export const catalogRevisions = sqliteTable('catalog_revisions', { + workspaceId: text('workspace_id').primaryKey().default('default'), + revision: integer('revision').notNull().default(0) +}); + +// Durable operation/outbox row for the committed-catalog-write contract +// (workspace-sharding.md §4). Phase A only ever writes status:'published' +// rows, in the same transaction as the catalog row they describe — the +// pending_content/content_durable/publishable states exist in the schema now +// (avoiding a later migration) but have no producer or consumer until a real +// cross-shard operation exists (Phase B) and an SSE drain exists (Phase C). +export const catalogOutbox = sqliteTable('catalog_outbox', { + id: integer('id').primaryKey({ autoIncrement: true }), + workspaceId: text('workspace_id').notNull().default('default'), + revision: integer('revision').notNull(), + status: text('status') + .notNull() + .$type<'pending_content' | 'content_durable' | 'publishable' | 'published'>(), + operationId: text('operation_id').notNull(), + payload: text('payload_json', { mode: 'json' }).notNull().$type<{ + documents?: string[]; + collections?: string[]; + op: 'create' | 'update' | 'move' | 'delete'; + }>(), + createdAt: integer('created_at').notNull(), + publishedAt: integer('published_at') +}); diff --git a/src/lib/server/workspace-store.test.ts b/src/lib/server/workspace-store.test.ts index 221cfe7..4ab6fcb 100644 --- a/src/lib/server/workspace-store.test.ts +++ b/src/lib/server/workspace-store.test.ts @@ -160,4 +160,14 @@ describe('workspace-store: isolation between independently-resolved contexts', ( expect(releaseContextIfIdle('space-a', 'main')).toBe(false); }); + + it('bootstraps a stable defaultSpaceId per workspace, distinct across workspaces', () => { + const a1 = resolveWorkspaceContext({ workspaceId: 'space-a', shardId: 'main' }); + const a2 = resolveWorkspaceContext({ workspaceId: 'space-a', shardId: 'main' }); + const b = resolveWorkspaceContext({ workspaceId: 'space-b', shardId: 'main' }); + + expect(a1.defaultSpaceId).toBeTruthy(); + expect(a2.defaultSpaceId).toBe(a1.defaultSpaceId); + expect(b.defaultSpaceId).not.toBe(a1.defaultSpaceId); + }); }); diff --git a/src/lib/server/workspace-store.ts b/src/lib/server/workspace-store.ts index 732dc00..6521eb4 100644 --- a/src/lib/server/workspace-store.ts +++ b/src/lib/server/workspace-store.ts @@ -7,6 +7,7 @@ import { resetAuditObserverForTests } from './audit-observer.js'; import { initHoldEviction, resetHoldEvictionForTests } from './holds.js'; +import { ensureCatalogBootstrapped } from './catalog.js'; // This is the one place a {workspaceId, shardId} selector resolves to a live // Y.Doc/Awareness/persistence/connection bundle. Every boundary that used to @@ -49,6 +50,8 @@ export interface WorkspaceContext { readonly awareness: Awareness; /** Live WebSocket connections currently bound to this context — see registerConnection/unregisterConnection. */ readonly connections: ReadonlySet; + /** The catalog Space this context's Documents/Collections are bootstrapped into — see ./catalog.ts. */ + readonly defaultSpaceId: string; } interface InternalContext extends WorkspaceContext { @@ -83,8 +86,11 @@ function createContext(workspaceId: string, shardId: string): InternalContext { if (snapshot) { Y.applyUpdate(doc, snapshot); } - // Attached only after the snapshot load above, so replaying prior state on - // process start never produces a spurious audit trail for it. + // Backfills/bootstraps the catalog from this doc's current content the + // first time this {workspaceId, shardId} resolves — see catalog.ts. Runs + // after the snapshot load (so it sees real content) and before the audit + // observer attaches (so it never produces a spurious audit trail). + const { defaultSpaceId } = ensureCatalogBootstrapped(workspaceId, shardId, doc); attachDocAuditObserver(doc); const awareness = new Awareness(doc); @@ -95,6 +101,7 @@ function createContext(workspaceId: string, shardId: string): InternalContext { shardId, doc, awareness, + defaultSpaceId, connections: new Set(), saveTimer: null, dirty: false diff --git a/src/lib/services/collections.ts b/src/lib/services/collections.ts index f8ab487..acd3e90 100644 --- a/src/lib/services/collections.ts +++ b/src/lib/services/collections.ts @@ -8,8 +8,15 @@ import { updateCollectionTitle as crdtUpdateCollectionTitle } from '$lib/data/records'; import { logAudit } from '$lib/server/audit'; +import { + recordCatalogCollectionCreated, + recordCatalogCollectionDeleted, + recordCatalogCollectionTitleChanged, + reserveCollectionLocator +} from '$lib/server/catalog'; import { grantCollectionAccess, tokenAllowsParent } from '$lib/mcp/tokens'; import type { CollectionMeta, PropertyDefinition, WorkspaceRecord } from '$lib/data/types'; +import { nanoid } from 'nanoid'; import { actorForCaller, isAccessToken, @@ -27,15 +34,26 @@ export function createCollection( caller: CallerIdentity, input: CreateCollectionInput ): CollectionMeta { - const { doc } = resolveWorkspaceContext(); + const { doc, workspaceId, shardId, defaultSpaceId } = resolveWorkspaceContext(); const actor = actorForCaller(caller); + const id = input.id ?? nanoid(); + reserveCollectionLocator(workspaceId, defaultSpaceId, id, shardId); + const collection = crdtCreateCollection(doc, { - id: input.id, + id, title: input.title, schema: input.schema ?? [] }); + recordCatalogCollectionCreated({ + workspaceId, + spaceId: defaultSpaceId, + id: collection.id, + title: collection.title, + shardId + }); + if (isAccessToken(caller)) { grantCollectionAccess(caller.tokenHash, collection.id); if (!caller.allowedCollectionIds.includes(collection.id)) { @@ -75,11 +93,12 @@ export function queryCollection( } export function deleteCollection(caller: CallerIdentity, collectionId: string): void { - const { doc } = resolveWorkspaceContext(); + const { doc, workspaceId } = resolveWorkspaceContext(); const actor = actorForCaller(caller); requireAccessibleParent(caller, collectionId, 'delete_collection'); crdtDeleteCollection(doc, collectionId); + recordCatalogCollectionDeleted(workspaceId, collectionId); logAudit({ actor, action: 'delete_collection', targetRecordId: collectionId }); } @@ -88,11 +107,12 @@ export function updateCollectionTitle( collectionId: string, title: string ): void { - const { doc } = resolveWorkspaceContext(); + const { doc, workspaceId } = resolveWorkspaceContext(); const actor = actorForCaller(caller); requireAccessibleParent(caller, collectionId, 'update_collection_title'); crdtUpdateCollectionTitle(doc, collectionId, title); + recordCatalogCollectionTitleChanged(workspaceId, collectionId, title); logAudit({ actor, action: 'update_collection_title', diff --git a/src/lib/services/documents.ts b/src/lib/services/documents.ts index 770ca66..46440c2 100644 --- a/src/lib/services/documents.ts +++ b/src/lib/services/documents.ts @@ -10,10 +10,18 @@ import { updateDocumentTitle as crdtUpdateDocumentTitle } from '$lib/data/records'; import { logAudit } from '$lib/server/audit'; +import { + recordCatalogDocumentCreated, + recordCatalogDocumentDeleted, + recordCatalogDocumentMoved, + recordCatalogDocumentTitleChanged, + reserveDocumentLocator +} from '$lib/server/catalog'; import { grantDocumentAccess, tokenAllowsParent } from '$lib/mcp/tokens'; import { richTextToMarkdown } from '$lib/mcp/markdown-transcode'; import { resolveInternalLinkTarget } from '$lib/data/links'; import type { DocumentMeta, EmbeddedViewConfig } from '$lib/data/types'; +import { nanoid } from 'nanoid'; import { actorForCaller, isAccessToken, @@ -30,7 +38,7 @@ export interface CreateDocumentInput { } export function createDocument(caller: CallerIdentity, input: CreateDocumentInput): DocumentMeta { - const { doc } = resolveWorkspaceContext(); + const { doc, workspaceId, shardId, defaultSpaceId } = resolveWorkspaceContext(); const actor = actorForCaller(caller); // Decision: In single-tenant Phase 0/1, any authenticated caller is permitted @@ -39,13 +47,30 @@ export function createDocument(caller: CallerIdentity, input: CreateDocumentInpu requireAccessibleParent(caller, input.parentDocumentId, 'create_document'); } + // Reserve the id in the catalog's workspace-wide record locator *before* + // any Y.Doc content is written — throws RecordIdConflictError on a + // collision instead of the Y.Doc primitive's prior silent overwrite (see + // docs/specifications/workspace-sharding.md §3.1). + const id = input.id ?? nanoid(); + reserveDocumentLocator(workspaceId, defaultSpaceId, id, shardId); + const document = crdtCreateDocument(doc, { - id: input.id, + id, title: input.title, parentDocumentId: input.parentDocumentId, afterDocumentId: input.afterDocumentId }); + recordCatalogDocumentCreated({ + workspaceId, + spaceId: defaultSpaceId, + id: document.id, + title: document.title, + parentDocumentId: document.parentDocumentId, + order: document.order, + shardId + }); + if (input.createInitialBlock) { crdtCreateRecord(doc, { parentId: document.id, blockType: 'paragraph' }, actor); } @@ -67,7 +92,7 @@ export function moveDocument( documentId: string, options: { parentDocumentId?: string; afterDocumentId?: string } ): void { - const { doc } = resolveWorkspaceContext(); + const { doc, workspaceId } = resolveWorkspaceContext(); const actor = actorForCaller(caller); requireAccessibleParent(caller, documentId, 'move_document'); @@ -76,6 +101,10 @@ export function moveDocument( } crdtUpdateDocumentParent(doc, documentId, options.parentDocumentId, options.afterDocumentId); + const moved = crdtGetDocument(doc, documentId); + if (moved) { + recordCatalogDocumentMoved(workspaceId, documentId, moved.parentDocumentId, moved.order); + } logAudit({ actor, action: 'move_document', @@ -85,11 +114,12 @@ export function moveDocument( } export function deleteDocument(caller: CallerIdentity, documentId: string): void { - const { doc } = resolveWorkspaceContext(); + const { doc, workspaceId } = resolveWorkspaceContext(); const actor = actorForCaller(caller); requireAccessibleParent(caller, documentId, 'delete_document'); crdtDeleteDocument(doc, documentId); + recordCatalogDocumentDeleted(workspaceId, documentId); logAudit({ actor, action: 'delete_document', targetRecordId: documentId }); } @@ -98,11 +128,12 @@ export function updateDocumentTitle( documentId: string, title: string ): void { - const { doc } = resolveWorkspaceContext(); + const { doc, workspaceId } = resolveWorkspaceContext(); const actor = actorForCaller(caller); requireAccessibleParent(caller, documentId, 'update_document_title'); crdtUpdateDocumentTitle(doc, documentId, title); + recordCatalogDocumentTitleChanged(workspaceId, documentId, title); logAudit({ actor, action: 'update_document_title', diff --git a/src/lib/services/services.test.ts b/src/lib/services/services.test.ts index f811c75..e8bef10 100644 --- a/src/lib/services/services.test.ts +++ b/src/lib/services/services.test.ts @@ -26,6 +26,11 @@ import { createToken, verifyToken } from '$lib/mcp/tokens'; import { queryAuditLog } from '$lib/server/audit'; import { resolveWorkspaceContext } from '$lib/server/workspace-store'; import { createRecord as crdtCreateRecord } from '$lib/data/records'; +import { + listCatalogCollections, + listCatalogDocuments, + RecordIdConflictError +} from '$lib/server/catalog'; import type { ActorId } from '$lib/data/types'; const human: ActorId = { kind: 'human', userId: 'brylie' }; @@ -759,3 +764,60 @@ describe('service layer: denied access attempts are themselves audited (docs/spe expect(entry?.diff).toBeUndefined(); }); }); + +describe('service layer: catalog stays in sync with Y.Doc document/collection mutations (#113 Phase A)', () => { + function catalogWorkspaceId(): string { + return resolveWorkspaceContext().workspaceId; + } + + it('mirrors document create, rename, move, and delete into the catalog', () => { + const parent = createDocument(human, { title: 'Catalog Parent' }); + const child = createDocument(human, { title: 'Catalog Child' }); + + let catalog = listCatalogDocuments(catalogWorkspaceId()); + expect(catalog.find((d) => d.id === parent.id)?.title).toBe('Catalog Parent'); + expect(catalog.find((d) => d.id === child.id)?.parentDocumentId).toBeUndefined(); + + updateDocumentTitle(human, child.id, 'Renamed Child'); + catalog = listCatalogDocuments(catalogWorkspaceId()); + expect(catalog.find((d) => d.id === child.id)?.title).toBe('Renamed Child'); + + moveDocument(human, child.id, { parentDocumentId: parent.id }); + catalog = listCatalogDocuments(catalogWorkspaceId()); + expect(catalog.find((d) => d.id === child.id)?.parentDocumentId).toBe(parent.id); + + deleteDocument(human, parent.id); + catalog = listCatalogDocuments(catalogWorkspaceId()); + expect(catalog.find((d) => d.id === parent.id)).toBeUndefined(); + expect(catalog.find((d) => d.id === child.id)).toBeUndefined(); // recursive descendant delete + }); + + it('mirrors collection create, rename, and delete into the catalog', () => { + const col = createCollection(human, { title: 'Catalog Table', schema: [] }); + + let catalog = listCatalogCollections(catalogWorkspaceId()); + expect(catalog.find((c) => c.id === col.id)?.title).toBe('Catalog Table'); + + updateCollectionTitle(human, col.id, 'Renamed Table'); + catalog = listCatalogCollections(catalogWorkspaceId()); + expect(catalog.find((c) => c.id === col.id)?.title).toBe('Renamed Table'); + + deleteCollection(human, col.id); + catalog = listCatalogCollections(catalogWorkspaceId()); + expect(catalog.find((c) => c.id === col.id)).toBeUndefined(); + }); + + it('rejects a caller-supplied document id that collides with an existing record', () => { + const existing = createDocument(human, { title: 'Existing' }); + expect(() => createDocument(human, { id: existing.id, title: 'Colliding' })).toThrow( + RecordIdConflictError + ); + }); + + it('rejects a caller-supplied collection id that collides with an existing document', () => { + const existingDoc = createDocument(human, { title: 'Existing Doc' }); + expect(() => + createCollection(human, { id: existingDoc.id, title: 'Colliding Collection', schema: [] }) + ).toThrow(RecordIdConflictError); + }); +}); diff --git a/src/routes/+layout.server.ts b/src/routes/+layout.server.ts index fd6d2e5..aa6cd2d 100644 --- a/src/routes/+layout.server.ts +++ b/src/routes/+layout.server.ts @@ -1,10 +1,11 @@ -import { listCollections, listDocuments } from '$lib/services'; -import { CURRENT_USER } from '$lib/server/current-user'; +import { listCatalogCollections, listCatalogDocuments } from '$lib/server/catalog'; +import { resolveWorkspaceContext } from '$lib/server/workspace-store'; import type { LayoutServerLoad } from './$types'; export const load: LayoutServerLoad = () => { + const { workspaceId } = resolveWorkspaceContext(); return { - documents: listDocuments(CURRENT_USER), - collections: listCollections(CURRENT_USER) + documents: listCatalogDocuments(workspaceId), + collections: listCatalogCollections(workspaceId) }; }; diff --git a/src/routes/+page.server.ts b/src/routes/+page.server.ts index 4e92585..184bc31 100644 --- a/src/routes/+page.server.ts +++ b/src/routes/+page.server.ts @@ -1,12 +1,15 @@ import { fail, redirect } from '@sveltejs/kit'; -import { createCollection, createDocument, listCollections, listDocuments } from '$lib/services'; +import { createCollection, createDocument } from '$lib/services'; import { CURRENT_USER } from '$lib/server/current-user'; +import { listCatalogCollections, listCatalogDocuments } from '$lib/server/catalog'; +import { resolveWorkspaceContext } from '$lib/server/workspace-store'; import type { Actions, PageServerLoad } from './$types'; export const load: PageServerLoad = () => { + const { workspaceId } = resolveWorkspaceContext(); return { - documents: listDocuments(CURRENT_USER), - collections: listCollections(CURRENT_USER) + documents: listCatalogDocuments(workspaceId), + collections: listCatalogCollections(workspaceId) }; }; From 2e883e489e7d35ed1298c8172afe22b85e93ff75 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Sun, 30 Aug 2026 18:59:12 +0300 Subject: [PATCH 2/4] fix: address CodeRabbit findings on catalog commit atomicity, id collisions, and cross-workspace scoping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Thread one shared transaction through each catalog lifecycle mutation, its revision bump, and its outbox insert (previously separate calls, some not transacted at all) — a crash could leave a committed catalog change with no matching revision/outbox event. - createDocument/createCollection now also check the live Y.Doc for existing content at a caller-supplied id, not just the SQL locator — closes the gap where an id colliding with content written directly to the Y.Doc (bypassing the service layer) would still silently overwrite it. Narrower than the reviewer's suggested full reconciliation, which is Phase B/C/D-scale machinery already documented as out of scope here. - catalog_documents/catalog_collections now use a (workspaceId, id) composite primary key instead of a bare global id, matching record_locator's own (workspaceId, recordId) scoping — a bare id PK would let workspace A's locator reservation succeed and then throw an unhandled SQL error the moment workspace B tried the same id. 648/648 tests passing (4 new, covering all three fixes). Refs #113 --- ...ty_kid_colt.sql => 0002_stormy_menace.sql} | 6 +- drizzle/meta/0002_snapshot.json | 26 ++++- drizzle/meta/_journal.json | 4 +- src/lib/server/catalog.test.ts | 101 +++++++++++++++++- src/lib/server/catalog.ts | 86 +++++++++------ src/lib/server/db/schema.ts | 83 ++++++++------ src/lib/services/collections.ts | 8 ++ src/lib/services/documents.ts | 10 +- src/lib/services/services.test.ts | 35 +++++- 9 files changed, 278 insertions(+), 81 deletions(-) rename drizzle/{0002_salty_kid_colt.sql => 0002_stormy_menace.sql} (94%) diff --git a/drizzle/0002_salty_kid_colt.sql b/drizzle/0002_stormy_menace.sql similarity index 94% rename from drizzle/0002_salty_kid_colt.sql rename to drizzle/0002_stormy_menace.sql index a2d8dc3..9068804 100644 --- a/drizzle/0002_salty_kid_colt.sql +++ b/drizzle/0002_stormy_menace.sql @@ -1,16 +1,17 @@ CREATE TABLE `catalog_collections` ( - `id` text PRIMARY KEY NOT NULL, + `id` text NOT NULL, `workspace_id` text DEFAULT 'default' NOT NULL, `space_id` text NOT NULL, `shard_id` text DEFAULT 'default' NOT NULL, `title` text NOT NULL, `created_at` integer NOT NULL, `updated_at` integer NOT NULL, + PRIMARY KEY(`workspace_id`, `id`), FOREIGN KEY (`space_id`) REFERENCES `spaces`(`id`) ON UPDATE no action ON DELETE no action ); --> statement-breakpoint CREATE TABLE `catalog_documents` ( - `id` text PRIMARY KEY NOT NULL, + `id` text NOT NULL, `workspace_id` text DEFAULT 'default' NOT NULL, `space_id` text NOT NULL, `shard_id` text DEFAULT 'default' NOT NULL, @@ -19,6 +20,7 @@ CREATE TABLE `catalog_documents` ( `order` text NOT NULL, `created_at` integer NOT NULL, `updated_at` integer NOT NULL, + PRIMARY KEY(`workspace_id`, `id`), FOREIGN KEY (`space_id`) REFERENCES `spaces`(`id`) ON UPDATE no action ON DELETE no action ); --> statement-breakpoint diff --git a/drizzle/meta/0002_snapshot.json b/drizzle/meta/0002_snapshot.json index 0ebe15e..e2c36f3 100644 --- a/drizzle/meta/0002_snapshot.json +++ b/drizzle/meta/0002_snapshot.json @@ -1,7 +1,7 @@ { "version": "6", "dialect": "sqlite", - "id": "7b84718e-e792-4956-a333-a01efe8f0485", + "id": "f2e87a44-6451-4ed1-baea-48026ac2b2e8", "prevId": "ae36812a-00d1-455c-acae-ba1fde05dbb1", "tables": { "access_tokens": { @@ -114,7 +114,7 @@ "id": { "name": "id", "type": "text", - "primaryKey": true, + "primaryKey": false, "notNull": true, "autoincrement": false }, @@ -179,7 +179,15 @@ "onUpdate": "no action" } }, - "compositePrimaryKeys": {}, + "compositePrimaryKeys": { + "catalog_collections_workspace_id_id_pk": { + "columns": [ + "workspace_id", + "id" + ], + "name": "catalog_collections_workspace_id_id_pk" + } + }, "uniqueConstraints": {}, "checkConstraints": {} }, @@ -189,7 +197,7 @@ "id": { "name": "id", "type": "text", - "primaryKey": true, + "primaryKey": false, "notNull": true, "autoincrement": false }, @@ -268,7 +276,15 @@ "onUpdate": "no action" } }, - "compositePrimaryKeys": {}, + "compositePrimaryKeys": { + "catalog_documents_workspace_id_id_pk": { + "columns": [ + "workspace_id", + "id" + ], + "name": "catalog_documents_workspace_id_id_pk" + } + }, "uniqueConstraints": {}, "checkConstraints": {} }, diff --git a/drizzle/meta/_journal.json b/drizzle/meta/_journal.json index d030f03..310b2a6 100644 --- a/drizzle/meta/_journal.json +++ b/drizzle/meta/_journal.json @@ -19,8 +19,8 @@ { "idx": 2, "version": "6", - "when": 1788103543215, - "tag": "0002_salty_kid_colt", + "when": 1788105346147, + "tag": "0002_stormy_menace", "breakpoints": true } ] diff --git a/src/lib/server/catalog.test.ts b/src/lib/server/catalog.test.ts index e9fd2d7..9792929 100644 --- a/src/lib/server/catalog.test.ts +++ b/src/lib/server/catalog.test.ts @@ -16,7 +16,11 @@ import { recordCatalogDocumentMoved, recordCatalogDocumentTitleChanged, RecordIdConflictError, - reserveDocumentLocator + reserveDocumentLocator, + reserveCollectionLocator, + recordCatalogCollectionCreated, + recordCatalogCollectionTitleChanged, + recordCatalogCollectionDeleted } from './catalog'; const WS = 'default'; @@ -232,3 +236,98 @@ describe('catalog: document deletion cascades to descendants', () => { expect(row?.title).toBe('After'); }); }); + +describe('catalog: two workspaces reusing the same record id stay isolated', () => { + // record_locator scopes uniqueness to (workspaceId, recordId) — a bare + // global `id` primary key on catalog_documents/catalog_collections would + // let workspace A's reservation succeed, then throw an unhandled SQL + // error the moment workspace B tried to insert its own row with the same + // id. This proves the composite (workspaceId, id) primary key actually + // allows that, and that mutating one workspace's row never touches the + // other's. + it('lets two workspaces each create a Document with the same id, and update/delete only affects the right one', () => { + const docA = new Y.Doc(); + const { defaultSpaceId: spaceA } = ensureCatalogBootstrapped('workspace-a', SHARD, docA); + const docB = new Y.Doc(); + const { defaultSpaceId: spaceB } = ensureCatalogBootstrapped('workspace-b', SHARD, docB); + + reserveDocumentLocator('workspace-a', spaceA, 'shared-id', SHARD); + recordCatalogDocumentCreated({ + workspaceId: 'workspace-a', + spaceId: spaceA, + id: 'shared-id', + title: 'Workspace A Doc', + order: 'a0', + shardId: SHARD + }); + + expect(() => { + reserveDocumentLocator('workspace-b', spaceB, 'shared-id', SHARD); + recordCatalogDocumentCreated({ + workspaceId: 'workspace-b', + spaceId: spaceB, + id: 'shared-id', + title: 'Workspace B Doc', + order: 'a0', + shardId: SHARD + }); + }).not.toThrow(); + + recordCatalogDocumentTitleChanged('workspace-a', 'shared-id', 'Renamed A'); + expect(listCatalogDocuments('workspace-a').find((d) => d.id === 'shared-id')?.title).toBe( + 'Renamed A' + ); + expect(listCatalogDocuments('workspace-b').find((d) => d.id === 'shared-id')?.title).toBe( + 'Workspace B Doc' + ); + + recordCatalogDocumentDeleted('workspace-a', 'shared-id'); + expect(listCatalogDocuments('workspace-a').find((d) => d.id === 'shared-id')).toBeUndefined(); + expect(listCatalogDocuments('workspace-b').find((d) => d.id === 'shared-id')?.title).toBe( + 'Workspace B Doc' + ); + }); + + it('lets two workspaces each create a Collection with the same id, and update/delete only affects the right one', () => { + const docA = new Y.Doc(); + const { defaultSpaceId: spaceA } = ensureCatalogBootstrapped('workspace-c', SHARD, docA); + const docB = new Y.Doc(); + const { defaultSpaceId: spaceB } = ensureCatalogBootstrapped('workspace-d', SHARD, docB); + + reserveCollectionLocator('workspace-c', spaceA, 'shared-collection-id', SHARD); + recordCatalogCollectionCreated({ + workspaceId: 'workspace-c', + spaceId: spaceA, + id: 'shared-collection-id', + title: 'Workspace C Table', + shardId: SHARD + }); + + expect(() => { + reserveCollectionLocator('workspace-d', spaceB, 'shared-collection-id', SHARD); + recordCatalogCollectionCreated({ + workspaceId: 'workspace-d', + spaceId: spaceB, + id: 'shared-collection-id', + title: 'Workspace D Table', + shardId: SHARD + }); + }).not.toThrow(); + + recordCatalogCollectionTitleChanged('workspace-c', 'shared-collection-id', 'Renamed C'); + expect( + listCatalogCollections('workspace-c').find((c) => c.id === 'shared-collection-id')?.title + ).toBe('Renamed C'); + expect( + listCatalogCollections('workspace-d').find((c) => c.id === 'shared-collection-id')?.title + ).toBe('Workspace D Table'); + + recordCatalogCollectionDeleted('workspace-c', 'shared-collection-id'); + expect( + listCatalogCollections('workspace-c').find((c) => c.id === 'shared-collection-id') + ).toBeUndefined(); + expect( + listCatalogCollections('workspace-d').find((c) => c.id === 'shared-collection-id')?.title + ).toBe('Workspace D Table'); + }); +}); diff --git a/src/lib/server/catalog.ts b/src/lib/server/catalog.ts index efcb69a..9f9d09e 100644 --- a/src/lib/server/catalog.ts +++ b/src/lib/server/catalog.ts @@ -2,6 +2,7 @@ import * as Y from 'yjs'; import { and, eq, inArray, sql } from 'drizzle-orm'; import { nanoid } from 'nanoid'; import { getDb } from './store.js'; +import type { Db } from './db/index.js'; import { catalogCollections, catalogDocuments, @@ -16,6 +17,11 @@ import { } from '../data/records.js'; import type { CollectionMeta, DocumentMeta, ParentKind } from '../data/types.js'; +// The transaction handle drizzle's better-sqlite3 driver passes into a +// db.transaction(...) callback — extracted from Db['transaction'] itself +// rather than hand-typed, so it can't drift from the real type. +type Tx = Parameters[0]>[0]; + // The catalog: durable SQLite metadata fronting the (today: single, Phase B: // per-Document/per-Collection) Y.Doc content shard(s) — see // docs/specifications/workspace-sharding.md §3.1. Phase A dual-writes this @@ -33,12 +39,20 @@ export class RecordIdConflictError extends Error { type CatalogOp = 'create' | 'update' | 'move' | 'delete'; +/** + * Bumps the workspace's catalog revision and appends its outbox row. + * Takes the caller's own transaction handle rather than opening one itself, + * so a crash between the catalog mutation, the revision bump, and the + * outbox insert can never leave any of the three committed without the + * others — the committed-write contract (workspace-sharding.md §4) this + * exists to guarantee would otherwise be undermined by exactly that gap. + */ function bumpRevisionAndAppendOutbox( + tx: Tx, workspaceId: string, payload: { documents?: string[]; collections?: string[]; op: CatalogOp } ): void { - const db = getDb(); - db.insert(catalogRevisions) + tx.insert(catalogRevisions) .values({ workspaceId, revision: 1 }) .onConflictDoUpdate({ target: catalogRevisions.workspaceId, @@ -46,13 +60,13 @@ function bumpRevisionAndAppendOutbox( }) .run(); const revision = - db + tx .select({ revision: catalogRevisions.revision }) .from(catalogRevisions) .where(eq(catalogRevisions.workspaceId, workspaceId)) .get()?.revision ?? 1; const now = Date.now(); - db.insert(catalogOutbox) + tx.insert(catalogOutbox) .values({ workspaceId, revision, @@ -120,9 +134,8 @@ export function recordCatalogDocumentCreated(input: { order: string; shardId: string; }): void { - const db = getDb(); const now = Date.now(); - db.transaction((tx) => { + getDb().transaction((tx) => { tx.insert(catalogDocuments) .values({ id: input.id, @@ -136,8 +149,8 @@ export function recordCatalogDocumentCreated(input: { updatedAt: now }) .run(); + bumpRevisionAndAppendOutbox(tx, input.workspaceId, { documents: [input.id], op: 'create' }); }); - bumpRevisionAndAppendOutbox(input.workspaceId, { documents: [input.id], op: 'create' }); } export function recordCatalogDocumentTitleChanged( @@ -145,12 +158,13 @@ export function recordCatalogDocumentTitleChanged( id: string, title: string ): void { - getDb() - .update(catalogDocuments) - .set({ title, updatedAt: Date.now() }) - .where(eq(catalogDocuments.id, id)) - .run(); - bumpRevisionAndAppendOutbox(workspaceId, { documents: [id], op: 'update' }); + getDb().transaction((tx) => { + tx.update(catalogDocuments) + .set({ title, updatedAt: Date.now() }) + .where(and(eq(catalogDocuments.workspaceId, workspaceId), eq(catalogDocuments.id, id))) + .run(); + bumpRevisionAndAppendOutbox(tx, workspaceId, { documents: [id], op: 'update' }); + }); } export function recordCatalogDocumentMoved( @@ -159,12 +173,13 @@ export function recordCatalogDocumentMoved( parentDocumentId: string | undefined, order: string ): void { - getDb() - .update(catalogDocuments) - .set({ parentDocumentId: parentDocumentId ?? null, order, updatedAt: Date.now() }) - .where(eq(catalogDocuments.id, id)) - .run(); - bumpRevisionAndAppendOutbox(workspaceId, { documents: [id], op: 'move' }); + getDb().transaction((tx) => { + tx.update(catalogDocuments) + .set({ parentDocumentId: parentDocumentId ?? null, order, updatedAt: Date.now() }) + .where(and(eq(catalogDocuments.workspaceId, workspaceId), eq(catalogDocuments.id, id))) + .run(); + bumpRevisionAndAppendOutbox(tx, workspaceId, { documents: [id], op: 'move' }); + }); } /** @@ -201,9 +216,11 @@ export function recordCatalogDocumentDeleted(workspaceId: string, id: string): v } // parentDocumentId isn't a real FK (see db/schema.ts), so every // descendant is deleted explicitly rather than relying on a cascade. - tx.delete(catalogDocuments).where(inArray(catalogDocuments.id, ids)).run(); + tx.delete(catalogDocuments) + .where(and(eq(catalogDocuments.workspaceId, workspaceId), inArray(catalogDocuments.id, ids))) + .run(); + bumpRevisionAndAppendOutbox(tx, workspaceId, { documents: ids, op: 'delete' }); }); - bumpRevisionAndAppendOutbox(workspaceId, { documents: ids, op: 'delete' }); } export function recordCatalogCollectionCreated(input: { @@ -213,9 +230,8 @@ export function recordCatalogCollectionCreated(input: { title: string; shardId: string; }): void { - const db = getDb(); const now = Date.now(); - db.transaction((tx) => { + getDb().transaction((tx) => { tx.insert(catalogCollections) .values({ id: input.id, @@ -227,8 +243,8 @@ export function recordCatalogCollectionCreated(input: { updatedAt: now }) .run(); + bumpRevisionAndAppendOutbox(tx, input.workspaceId, { collections: [input.id], op: 'create' }); }); - bumpRevisionAndAppendOutbox(input.workspaceId, { collections: [input.id], op: 'create' }); } export function recordCatalogCollectionTitleChanged( @@ -236,23 +252,25 @@ export function recordCatalogCollectionTitleChanged( id: string, title: string ): void { - getDb() - .update(catalogCollections) - .set({ title, updatedAt: Date.now() }) - .where(eq(catalogCollections.id, id)) - .run(); - bumpRevisionAndAppendOutbox(workspaceId, { collections: [id], op: 'update' }); + getDb().transaction((tx) => { + tx.update(catalogCollections) + .set({ title, updatedAt: Date.now() }) + .where(and(eq(catalogCollections.workspaceId, workspaceId), eq(catalogCollections.id, id))) + .run(); + bumpRevisionAndAppendOutbox(tx, workspaceId, { collections: [id], op: 'update' }); + }); } export function recordCatalogCollectionDeleted(workspaceId: string, id: string): void { - const db = getDb(); - db.transaction((tx) => { + getDb().transaction((tx) => { tx.delete(recordLocator) .where(and(eq(recordLocator.workspaceId, workspaceId), eq(recordLocator.recordId, id))) .run(); - tx.delete(catalogCollections).where(eq(catalogCollections.id, id)).run(); + tx.delete(catalogCollections) + .where(and(eq(catalogCollections.workspaceId, workspaceId), eq(catalogCollections.id, id))) + .run(); + bumpRevisionAndAppendOutbox(tx, workspaceId, { collections: [id], op: 'delete' }); }); - bumpRevisionAndAppendOutbox(workspaceId, { collections: [id], op: 'delete' }); } export function listCatalogDocuments(workspaceId: string): DocumentMeta[] { diff --git a/src/lib/server/db/schema.ts b/src/lib/server/db/schema.ts index a998705..78d3cae 100644 --- a/src/lib/server/db/schema.ts +++ b/src/lib/server/db/schema.ts @@ -1,4 +1,4 @@ -import { blob, integer, sqliteTable, text, uniqueIndex } from 'drizzle-orm/sqlite-core'; +import { blob, integer, primaryKey, sqliteTable, text, uniqueIndex } from 'drizzle-orm/sqlite-core'; import type { ActorId } from '$lib/data/types'; export const snapshots = sqliteTable('snapshots', { @@ -49,41 +49,54 @@ export const spaces = sqliteTable('spaces', { createdAt: integer('created_at').notNull() }); -export const catalogDocuments = sqliteTable('catalog_documents', { - id: text('id').primaryKey(), // == the Y.Doc DocumentMeta.id it mirrors - workspaceId: text('workspace_id').notNull().default('default'), - spaceId: text('space_id') - .notNull() - .references(() => spaces.id), - shardId: text('shard_id').notNull().default('default'), - title: text('title').notNull(), - // Deliberately NOT a foreign key: a Document can be created by a client - // writing directly to the Y.Doc over Yjs sync, bypassing the service layer - // entirely (a supported pattern — see docs/specifications/audit-coverage.md - // and tests/e2e/tier-a.test.ts's direct-Yjs-client cases). Its catalog row - // wouldn't exist yet, so a strict FK on a real parentDocumentId would throw - // on an otherwise-valid nested create. recordCatalogDocumentDeleted (see - // catalog.ts) therefore deletes descendants explicitly rather than relying - // on ON DELETE CASCADE. - parentDocumentId: text('parent_document_id'), - order: text('order').notNull(), // mirrors DocumentMeta.order exactly, never independently recomputed - createdAt: integer('created_at').notNull(), - updatedAt: integer('updated_at').notNull() -}); +// Primary key is (workspaceId, id), not bare id: record_locator scopes +// uniqueness the same way (a recordId is only unique *within* a workspace), +// so a bare global id PK here would throw on an otherwise-valid second +// workspace reusing the same id — the locator would have already accepted +// the reservation. +export const catalogDocuments = sqliteTable( + 'catalog_documents', + { + id: text('id').notNull(), // == the Y.Doc DocumentMeta.id it mirrors + workspaceId: text('workspace_id').notNull().default('default'), + spaceId: text('space_id') + .notNull() + .references(() => spaces.id), + shardId: text('shard_id').notNull().default('default'), + title: text('title').notNull(), + // Deliberately NOT a foreign key: a Document can be created by a client + // writing directly to the Y.Doc over Yjs sync, bypassing the service layer + // entirely (a supported pattern — see docs/specifications/audit-coverage.md + // and tests/e2e/tier-a.test.ts's direct-Yjs-client cases). Its catalog row + // wouldn't exist yet, so a strict FK on a real parentDocumentId would throw + // on an otherwise-valid nested create. recordCatalogDocumentDeleted (see + // catalog.ts) therefore deletes descendants explicitly rather than relying + // on ON DELETE CASCADE. + parentDocumentId: text('parent_document_id'), + order: text('order').notNull(), // mirrors DocumentMeta.order exactly, never independently recomputed + createdAt: integer('created_at').notNull(), + updatedAt: integer('updated_at').notNull() + }, + (t) => [primaryKey({ columns: [t.workspaceId, t.id] })] +); -export const catalogCollections = sqliteTable('catalog_collections', { - id: text('id').primaryKey(), // == the Y.Doc CollectionMeta.id it mirrors - workspaceId: text('workspace_id').notNull().default('default'), - spaceId: text('space_id') - .notNull() - .references(() => spaces.id), - shardId: text('shard_id').notNull().default('default'), - title: text('title').notNull(), - // No parent/order (Collections are flat) and no schema mirror — schema - // stays shard-owned per workspace-sharding.md §3.1/§3.2. - createdAt: integer('created_at').notNull(), - updatedAt: integer('updated_at').notNull() -}); +export const catalogCollections = sqliteTable( + 'catalog_collections', + { + id: text('id').notNull(), // == the Y.Doc CollectionMeta.id it mirrors + workspaceId: text('workspace_id').notNull().default('default'), + spaceId: text('space_id') + .notNull() + .references(() => spaces.id), + shardId: text('shard_id').notNull().default('default'), + title: text('title').notNull(), + // No parent/order (Collections are flat) and no schema mirror — schema + // stays shard-owned per workspace-sharding.md §3.1/§3.2. + createdAt: integer('created_at').notNull(), + updatedAt: integer('updated_at').notNull() + }, + (t) => [primaryKey({ columns: [t.workspaceId, t.id] })] +); // The workspace-wide (workspace_id, record_id) locator required by §3.1: the // mechanism that actually rejects a duplicate id across Documents/Collections diff --git a/src/lib/services/collections.ts b/src/lib/services/collections.ts index acd3e90..8ed7e84 100644 --- a/src/lib/services/collections.ts +++ b/src/lib/services/collections.ts @@ -9,6 +9,7 @@ import { } from '$lib/data/records'; import { logAudit } from '$lib/server/audit'; import { + RecordIdConflictError, recordCatalogCollectionCreated, recordCatalogCollectionDeleted, recordCatalogCollectionTitleChanged, @@ -38,6 +39,13 @@ export function createCollection( const actor = actorForCaller(caller); const id = input.id ?? nanoid(); + // See documents.ts's createDocument for why this also checks the live + // Y.Doc, not just the catalog locator: a caller-supplied id could collide + // with a Collection created by a client writing directly to the Y.Doc, + // bypassing the service layer (and therefore the locator) entirely. + if (crdtGetCollection(doc, id)) { + throw new RecordIdConflictError(id); + } reserveCollectionLocator(workspaceId, defaultSpaceId, id, shardId); const collection = crdtCreateCollection(doc, { diff --git a/src/lib/services/documents.ts b/src/lib/services/documents.ts index 46440c2..3ab972e 100644 --- a/src/lib/services/documents.ts +++ b/src/lib/services/documents.ts @@ -11,6 +11,7 @@ import { } from '$lib/data/records'; import { logAudit } from '$lib/server/audit'; import { + RecordIdConflictError, recordCatalogDocumentCreated, recordCatalogDocumentDeleted, recordCatalogDocumentMoved, @@ -50,8 +51,15 @@ export function createDocument(caller: CallerIdentity, input: CreateDocumentInpu // Reserve the id in the catalog's workspace-wide record locator *before* // any Y.Doc content is written — throws RecordIdConflictError on a // collision instead of the Y.Doc primitive's prior silent overwrite (see - // docs/specifications/workspace-sharding.md §3.1). + // docs/specifications/workspace-sharding.md §3.1). The locator alone + // can't catch a collision with content a client wrote directly to the + // Y.Doc (bypassing the service layer, and therefore the locator) — a + // caller-supplied id is checked against the live Y.Doc too, since + // crdtCreateDocument would otherwise silently overwrite it. const id = input.id ?? nanoid(); + if (crdtGetDocument(doc, id)) { + throw new RecordIdConflictError(id); + } reserveDocumentLocator(workspaceId, defaultSpaceId, id, shardId); const document = crdtCreateDocument(doc, { diff --git a/src/lib/services/services.test.ts b/src/lib/services/services.test.ts index e8bef10..bc0edc1 100644 --- a/src/lib/services/services.test.ts +++ b/src/lib/services/services.test.ts @@ -25,7 +25,13 @@ import { import { createToken, verifyToken } from '$lib/mcp/tokens'; import { queryAuditLog } from '$lib/server/audit'; import { resolveWorkspaceContext } from '$lib/server/workspace-store'; -import { createRecord as crdtCreateRecord } from '$lib/data/records'; +import { + createRecord as crdtCreateRecord, + createDocument as crdtCreateDocument, + createCollection as crdtCreateCollection, + getDocument as crdtGetDocument, + getCollection as crdtGetCollection +} from '$lib/data/records'; import { listCatalogCollections, listCatalogDocuments, @@ -820,4 +826,31 @@ describe('service layer: catalog stays in sync with Y.Doc document/collection mu createCollection(human, { id: existingDoc.id, title: 'Colliding Collection', schema: [] }) ).toThrow(RecordIdConflictError); }); + + it('rejects a caller-supplied id colliding with a document written directly to the Y.Doc, bypassing the service layer (never overwrites it)', () => { + const { doc } = resolveWorkspaceContext(); + // Simulates a real Yjs client writing straight to the Y.Doc — the + // locator/catalog never learn about this id, since it never went + // through reserveDocumentLocator/recordCatalogDocumentCreated. + const direct = crdtCreateDocument(doc, { title: 'Written Directly To The Y.Doc' }); + + expect(() => createDocument(human, { id: direct.id, title: 'Overwrite Attempt' })).toThrow( + RecordIdConflictError + ); + // The original content must survive untouched. + expect(crdtGetDocument(doc, direct.id)?.title).toBe('Written Directly To The Y.Doc'); + }); + + it('rejects a caller-supplied id colliding with a collection written directly to the Y.Doc, bypassing the service layer (never overwrites it)', () => { + const { doc } = resolveWorkspaceContext(); + const direct = crdtCreateCollection(doc, { + title: 'Written Directly To The Y.Doc', + schema: [] + }); + + expect(() => + createCollection(human, { id: direct.id, title: 'Overwrite Attempt', schema: [] }) + ).toThrow(RecordIdConflictError); + expect(crdtGetCollection(doc, direct.id)?.title).toBe('Written Directly To The Y.Doc'); + }); }); From 13a74cf02d23e25f27ac92aa133ddc967f7143af Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Sun, 30 Aug 2026 19:14:34 +0300 Subject: [PATCH 3/4] feat: make every service function shard-aware, without cutting over shard assignment yet (#120) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every service function that used to call resolveWorkspaceContext() bare now resolves its actual target shard via new catalog primitives (resolveShardForParent/resolveShardForRecord), falling back to the default context when untracked. createCollection still assigns shardId: 'default' deliberately — this proves the resolution mechanism correct for a genuinely separate shard (tests manually construct one, same pattern as the holds eviction-wiring fix) without changing where content actually lives yet, so production behavior is unchanged and no client/attach-ws changes are needed in this slice. - catalog.ts: reserveRecordLocator/releaseRecordLocator (row-level, closes the gap where write_record/delete_record/hold_records only ever receive a bare recordId), resolveShardForParent/resolveShardForRecord. - permissions.ts: resolveParentWorkspaceContext/resolveRecordWorkspaceContext/ groupRecordIdsByShard — shared resolution helpers; requireAccessibleRecord is now itself shard-aware, which every existing caller already goes through. - records.ts: createRecord reserves a row locator when its parent is a Collection; writeRecord/deleteRecord/getRecord resolve via the record's own locator. - collections.ts: queryCollection/updateCollectionTitle/deleteCollection resolve the collection's real shard. - holds.ts: hold_records/release_records group recordIds by resolved shard and operate against each shard's own Awareness (a cross-document agent batch is a stated acceptance criterion — see collaboration.md). - search.ts: Collections are enumerated via the catalog first (resolving each one's real shard, including its own meta entry — not just its rows), with a fallback pass over the default doc for uncataloged (direct-Yjs- written) Collections the catalog loop can't see. No MCP tool schema changes needed — every tool already carries enough of an id for server-side shard resolution. 661/661 tests passing (13 new). Refs #120. Branched off feat/workspace-catalog-113-phase-a (PR #119, not yet merged) since this depends on its catalog.ts. --- src/lib/server/catalog.test.ts | 59 +++++++++++- src/lib/server/catalog.ts | 63 ++++++++++++- src/lib/server/db/schema.ts | 6 +- src/lib/services/collections.ts | 7 +- src/lib/services/holds.ts | 33 +++++-- src/lib/services/permissions.ts | 48 +++++++++- src/lib/services/records.ts | 19 +++- src/lib/services/search.ts | 33 ++++++- src/lib/services/services.test.ts | 147 +++++++++++++++++++++++++++++- 9 files changed, 390 insertions(+), 25 deletions(-) diff --git a/src/lib/server/catalog.test.ts b/src/lib/server/catalog.test.ts index 9792929..d4d8794 100644 --- a/src/lib/server/catalog.test.ts +++ b/src/lib/server/catalog.test.ts @@ -20,7 +20,11 @@ import { reserveCollectionLocator, recordCatalogCollectionCreated, recordCatalogCollectionTitleChanged, - recordCatalogCollectionDeleted + recordCatalogCollectionDeleted, + reserveRecordLocator, + releaseRecordLocator, + resolveShardForParent, + resolveShardForRecord } from './catalog'; const WS = 'default'; @@ -331,3 +335,56 @@ describe('catalog: two workspaces reusing the same record id stay isolated', () ).toBe('Workspace D Table'); }); }); + +describe('catalog: record/row locator and shard resolution (#120)', () => { + it('resolveShardForParent finds a Document or Collection by its own id', () => { + const { defaultSpaceId } = bootstrap(); + reserveDocumentLocator(WS, defaultSpaceId, 'a-document', SHARD); + reserveCollectionLocator(WS, defaultSpaceId, 'a-collection', 'other-shard'); + + expect(resolveShardForParent(WS, 'a-document')).toEqual({ shardId: SHARD, kind: 'document' }); + expect(resolveShardForParent(WS, 'a-collection')).toEqual({ + shardId: 'other-shard', + kind: 'collection' + }); + }); + + it('resolveShardForParent returns undefined for an untracked id', () => { + bootstrap(); + expect(resolveShardForParent(WS, 'never-created')).toBeUndefined(); + }); + + it('reserves and resolves a record/row locator independently of Document/Collection locators', () => { + const { defaultSpaceId } = bootstrap(); + reserveRecordLocator(WS, defaultSpaceId, 'row-1', 'collection-shard-x'); + + expect(resolveShardForRecord(WS, 'row-1')).toEqual({ shardId: 'collection-shard-x' }); + // A record-kind locator entry must never satisfy a parent lookup — a + // row is never itself a valid parentId. + expect(resolveShardForParent(WS, 'row-1')).toBeUndefined(); + }); + + it('releaseRecordLocator removes the entry, and the id becomes reservable again', () => { + const { defaultSpaceId } = bootstrap(); + reserveRecordLocator(WS, defaultSpaceId, 'row-2', SHARD); + expect(resolveShardForRecord(WS, 'row-2')).toEqual({ shardId: SHARD }); + + releaseRecordLocator(WS, 'row-2'); + + expect(resolveShardForRecord(WS, 'row-2')).toBeUndefined(); + expect(() => reserveRecordLocator(WS, defaultSpaceId, 'row-2', SHARD)).not.toThrow(); + }); + + it('releaseRecordLocator on a never-reserved id is a safe no-op', () => { + bootstrap(); + expect(() => releaseRecordLocator(WS, 'never-reserved')).not.toThrow(); + }); + + it('rejects a duplicate record id reservation, consistent with Document/Collection locators', () => { + const { defaultSpaceId } = bootstrap(); + reserveRecordLocator(WS, defaultSpaceId, 'row-3', SHARD); + expect(() => reserveRecordLocator(WS, defaultSpaceId, 'row-3', SHARD)).toThrow( + RecordIdConflictError + ); + }); +}); diff --git a/src/lib/server/catalog.ts b/src/lib/server/catalog.ts index 9f9d09e..8c7e4b1 100644 --- a/src/lib/server/catalog.ts +++ b/src/lib/server/catalog.ts @@ -87,11 +87,13 @@ function bumpRevisionAndAppendOutbox( * silent-overwrite-on-duplicate-id behavior of data/records.ts's * createDocument/createCollection. */ +type LocatorKind = ParentKind | 'record'; + function reserveLocator( workspaceId: string, spaceId: string, recordId: string, - kind: ParentKind, + kind: LocatorKind, shardId: string ): void { try { @@ -125,6 +127,65 @@ export function reserveCollectionLocator( reserveLocator(workspaceId, spaceId, id, 'collection', shardId); } +/** + * Reserves a locator entry for one record/row within a sharded Collection — + * unlike Documents/Collections, individual records aren't catalog-navigable + * entities (§3.1), so this exists purely so write_record/delete_record/ + * hold_records/release_records (which only ever receive a bare recordId, no + * parent hint) can resolve which shard to operate against. Document blocks + * are never locator-tracked — they're always in the default shard as long + * as Documents themselves aren't sharded, so resolveShardForRecord's + * "not found" fallback already routes them correctly. + */ +export function reserveRecordLocator( + workspaceId: string, + spaceId: string, + recordId: string, + shardId: string +): void { + reserveLocator(workspaceId, spaceId, recordId, 'record', shardId); +} + +export function releaseRecordLocator(workspaceId: string, recordId: string): void { + getDb() + .delete(recordLocator) + .where(and(eq(recordLocator.workspaceId, workspaceId), eq(recordLocator.recordId, recordId))) + .run(); +} + +/** + * Resolves the shard a Document or Collection lives in, for callers that + * already have its own id (query_collection's collectionId, create_record's + * parentId). Returns undefined when untracked — content written directly to + * the Y.Doc, bypassing the service layer, or an id that doesn't exist — + * callers fall back to the default context in that case. + */ +export function resolveShardForParent( + workspaceId: string, + parentId: string +): { shardId: string; kind: 'document' | 'collection' } | undefined { + const row = getDb() + .select({ shardId: recordLocator.shardId, kind: recordLocator.kind }) + .from(recordLocator) + .where(and(eq(recordLocator.workspaceId, workspaceId), eq(recordLocator.recordId, parentId))) + .get(); + if (!row || row.kind === 'record') return undefined; + return { shardId: row.shardId, kind: row.kind }; +} + +/** Resolves the shard a single record/row lives in, for callers that only have a bare recordId. */ +export function resolveShardForRecord( + workspaceId: string, + recordId: string +): { shardId: string } | undefined { + const row = getDb() + .select({ shardId: recordLocator.shardId }) + .from(recordLocator) + .where(and(eq(recordLocator.workspaceId, workspaceId), eq(recordLocator.recordId, recordId))) + .get(); + return row ? { shardId: row.shardId } : undefined; +} + export function recordCatalogDocumentCreated(input: { workspaceId: string; spaceId: string; diff --git a/src/lib/server/db/schema.ts b/src/lib/server/db/schema.ts index 78d3cae..8379d6f 100644 --- a/src/lib/server/db/schema.ts +++ b/src/lib/server/db/schema.ts @@ -100,14 +100,16 @@ export const catalogCollections = sqliteTable( // The workspace-wide (workspace_id, record_id) locator required by §3.1: the // mechanism that actually rejects a duplicate id across Documents/Collections -// (today's separate Y.Maps for each don't prevent that at all). +// (today's separate Y.Maps for each don't prevent that at all). Also covers +// individual records/rows within a sharded Collection ('record' kind) — see +// reserveRecordLocator in catalog.ts. export const recordLocator = sqliteTable( 'record_locator', { id: integer('id').primaryKey({ autoIncrement: true }), workspaceId: text('workspace_id').notNull().default('default'), recordId: text('record_id').notNull(), - kind: text('kind').notNull().$type<'document' | 'collection'>(), + kind: text('kind').notNull().$type<'document' | 'collection' | 'record'>(), spaceId: text('space_id') .notNull() .references(() => spaces.id), diff --git a/src/lib/services/collections.ts b/src/lib/services/collections.ts index 8ed7e84..7adb51c 100644 --- a/src/lib/services/collections.ts +++ b/src/lib/services/collections.ts @@ -22,6 +22,7 @@ import { actorForCaller, isAccessToken, requireAccessibleParent, + resolveParentWorkspaceContext, type CallerIdentity } from './permissions'; @@ -89,7 +90,7 @@ export function queryCollection( collection: CollectionMeta | undefined; records: WorkspaceRecord[]; } { - const { doc } = resolveWorkspaceContext(); + const { doc } = resolveParentWorkspaceContext(collectionId); const actor = actorForCaller(caller); requireAccessibleParent(caller, collectionId, 'query_collection'); @@ -101,7 +102,7 @@ export function queryCollection( } export function deleteCollection(caller: CallerIdentity, collectionId: string): void { - const { doc, workspaceId } = resolveWorkspaceContext(); + const { doc, workspaceId } = resolveParentWorkspaceContext(collectionId); const actor = actorForCaller(caller); requireAccessibleParent(caller, collectionId, 'delete_collection'); @@ -115,7 +116,7 @@ export function updateCollectionTitle( collectionId: string, title: string ): void { - const { doc, workspaceId } = resolveWorkspaceContext(); + const { doc, workspaceId } = resolveParentWorkspaceContext(collectionId); const actor = actorForCaller(caller); requireAccessibleParent(caller, collectionId, 'update_collection_title'); diff --git a/src/lib/services/holds.ts b/src/lib/services/holds.ts index 2396316..88e52b0 100644 --- a/src/lib/services/holds.ts +++ b/src/lib/services/holds.ts @@ -5,26 +5,42 @@ import { logAudit } from '$lib/server/audit'; import { tokenAllowsParent } from '$lib/mcp/tokens'; import { actorForCaller, + groupRecordIdsByShard, isAccessToken, requireAccessibleRecord, type CallerIdentity } from './permissions'; +// A hold_records/release_records call can legitimately span more than one +// shard (a cross-document agent batch is a stated acceptance criterion — +// see docs/specifications/collaboration.md) — recordIds are grouped by +// their resolved shard, and requestAgentHold/releaseAgentHold run once per +// shard's own Awareness, merging results. In production every group +// resolves to the same default shard today (#120 hasn't cut over shard +// assignment yet), so this is a no-op split until it does. export function holdRecords( caller: CallerIdentity, recordIds: string[] ): { granted: string[]; denied: string[] } { - const { doc, awareness } = resolveWorkspaceContext(); const actor = actorForCaller(caller); let result: { granted: string[]; denied: string[] }; if (isAccessToken(caller)) { const clientId = clientIdForToken(caller.tokenHash); - result = requestAgentHold(awareness, clientId, actor, recordIds, (id) => { - const record = getRecord(doc, id); - return record ? tokenAllowsParent(caller, record.parentId) : false; - }); + const { workspaceId } = resolveWorkspaceContext(); + const granted: string[] = []; + const denied: string[] = []; + for (const [shardId, ids] of groupRecordIdsByShard(recordIds)) { + const { doc, awareness } = resolveWorkspaceContext({ workspaceId, shardId }); + const groupResult = requestAgentHold(awareness, clientId, actor, ids, (id) => { + const record = getRecord(doc, id); + return record ? tokenAllowsParent(caller, record.parentId) : false; + }); + granted.push(...groupResult.granted); + denied.push(...groupResult.denied); + } + result = { granted, denied }; } else { // Human callers: check record existence and permission const granted: string[] = []; @@ -45,12 +61,15 @@ export function holdRecords( } export function releaseRecords(caller: CallerIdentity, recordIds: string[]): void { - const { awareness } = resolveWorkspaceContext(); const actor = actorForCaller(caller); if (isAccessToken(caller)) { const clientId = clientIdForToken(caller.tokenHash); - releaseAgentHold(awareness, clientId, recordIds); + const { workspaceId } = resolveWorkspaceContext(); + for (const [shardId, ids] of groupRecordIdsByShard(recordIds)) { + const { awareness } = resolveWorkspaceContext({ workspaceId, shardId }); + releaseAgentHold(awareness, clientId, ids); + } } logAudit({ actor, action: 'release_records', diff: { recordIds } }); diff --git a/src/lib/services/permissions.ts b/src/lib/services/permissions.ts index 99efa4b..19a4c7f 100644 --- a/src/lib/services/permissions.ts +++ b/src/lib/services/permissions.ts @@ -1,8 +1,9 @@ import type { ActorId } from '$lib/data/types'; -import { resolveWorkspaceContext } from '$lib/server/workspace-store'; +import { resolveWorkspaceContext, type WorkspaceContext } from '$lib/server/workspace-store'; import { getRecord } from '$lib/data/records'; import { tokenAllowsParent, type AccessToken } from '$lib/mcp/tokens'; import { logAudit } from '$lib/server/audit'; +import { resolveShardForParent, resolveShardForRecord } from '$lib/server/catalog'; export type CallerIdentity = AccessToken | ActorId; @@ -57,7 +58,7 @@ export function requireAccessibleRecord( recordId: string, action?: string ): NonNullable> { - const { doc } = resolveWorkspaceContext(); + const { doc } = resolveRecordWorkspaceContext(recordId); const record = getRecord(doc, recordId); if (!record) { logDenial(caller, action, recordId); @@ -66,3 +67,46 @@ export function requireAccessibleRecord( requireAccessibleParent(caller, record.parentId, action); return record; } + +/** + * Resolves the WorkspaceContext a Document/Collection actually lives in, + * for callers that already have its own id (query_collection's + * collectionId, create_record's parentId) — see catalog.ts's + * resolveShardForParent. Falls back to the default context when untracked + * (content written directly to the Y.Doc, or a Document — Documents aren't + * sharded yet, so they're never locator-tracked). + */ +export function resolveParentWorkspaceContext( + parentId: string +): WorkspaceContext & { parentKind?: 'document' | 'collection' } { + const { workspaceId } = resolveWorkspaceContext(); + const shard = resolveShardForParent(workspaceId, parentId); + const ctx = resolveWorkspaceContext( + shard ? { workspaceId, shardId: shard.shardId } : { workspaceId } + ); + return { ...ctx, parentKind: shard?.kind }; +} + +/** + * Resolves the WorkspaceContext a single record/row lives in, for callers + * that only have a bare recordId (write_record, delete_record, get_record). + * See catalog.ts's resolveShardForRecord. + */ +export function resolveRecordWorkspaceContext(recordId: string): WorkspaceContext { + const { workspaceId } = resolveWorkspaceContext(); + const shard = resolveShardForRecord(workspaceId, recordId); + return resolveWorkspaceContext(shard ? { workspaceId, shardId: shard.shardId } : { workspaceId }); +} + +/** Groups recordIds by their resolved shard, for a hold/release call that may legitimately span more than one. */ +export function groupRecordIdsByShard(recordIds: string[]): Map { + const { workspaceId, shardId: defaultShardId } = resolveWorkspaceContext(); + const groups = new Map(); + for (const id of recordIds) { + const shardId = resolveShardForRecord(workspaceId, id)?.shardId ?? defaultShardId; + const list = groups.get(shardId); + if (list) list.push(id); + else groups.set(shardId, [id]); + } + return groups; +} diff --git a/src/lib/services/records.ts b/src/lib/services/records.ts index 36c907b..3a410d5 100644 --- a/src/lib/services/records.ts +++ b/src/lib/services/records.ts @@ -10,6 +10,7 @@ import { updateRecordProperties } from '$lib/data/records'; import { logAudit } from '$lib/server/audit'; +import { reserveRecordLocator, releaseRecordLocator } from '$lib/server/catalog'; import { markdownToRichText } from '$lib/mcp/markdown-transcode'; import { yTextToRichText } from '$lib/data/richtext'; import { tokenAllowsParent } from '$lib/mcp/tokens'; @@ -19,6 +20,8 @@ import { isAccessToken, requireAccessibleParent, requireAccessibleRecord, + resolveParentWorkspaceContext, + resolveRecordWorkspaceContext, type CallerIdentity } from './permissions'; @@ -61,7 +64,9 @@ export function createRecord( referencedRecordId?: string; } ): WorkspaceRecord { - const { doc } = resolveWorkspaceContext(); + const { doc, workspaceId, shardId, defaultSpaceId, parentKind } = resolveParentWorkspaceContext( + input.parentId + ); const actor = actorForCaller(caller); requireAccessibleParent(caller, input.parentId, 'create_record'); @@ -88,6 +93,13 @@ export function createRecord( actor ); + // Document blocks stay untracked — they're always in the default shard as + // long as Documents themselves aren't sharded, so resolveRecordWorkspaceContext's + // "not found" fallback already routes them correctly without a locator row. + if (parentKind === 'collection') { + reserveRecordLocator(workspaceId, defaultSpaceId, record.id, shardId); + } + logAudit({ actor, action: 'create_record', targetRecordId: record.id }); return record; } @@ -105,7 +117,7 @@ export function writeRecord( throw new Error('write_record requires markdown, properties, or referencedRecordId'); } - const { doc, awareness } = resolveWorkspaceContext(); + const { doc, awareness } = resolveRecordWorkspaceContext(recordId); const actor = actorForCaller(caller); const record = requireAccessibleRecord(caller, recordId, 'write_record'); @@ -188,11 +200,12 @@ export function writeRecord( } export function deleteRecord(caller: CallerIdentity, recordId: string): void { - const { doc } = resolveWorkspaceContext(); + const { doc, workspaceId } = resolveRecordWorkspaceContext(recordId); const actor = actorForCaller(caller); requireAccessibleRecord(caller, recordId, 'delete_record'); crdtDeleteRecord(doc, recordId); + releaseRecordLocator(workspaceId, recordId); logAudit({ actor, action: 'delete_record', targetRecordId: recordId }); } diff --git a/src/lib/services/search.ts b/src/lib/services/search.ts index 57d2c06..c3252a4 100644 --- a/src/lib/services/search.ts +++ b/src/lib/services/search.ts @@ -1,6 +1,7 @@ import { resolveWorkspaceContext } from '$lib/server/workspace-store'; import { listCollections, listDocuments, listRecordsForParent } from '$lib/data/records'; import { logAudit } from '$lib/server/audit'; +import { listCatalogCollections, resolveShardForParent } from '$lib/server/catalog'; import { tokenAllowsParent } from '$lib/mcp/tokens'; import { richTextToMarkdown } from '$lib/mcp/markdown-transcode'; import { actorForCaller, isAccessToken, type CallerIdentity } from './permissions'; @@ -17,7 +18,7 @@ export function searchWorkspace( caller: CallerIdentity, query: string ): Array<{ recordId: string; snippet: string }> { - const { doc } = resolveWorkspaceContext(); + const { doc, workspaceId } = resolveWorkspaceContext(); const actor = actorForCaller(caller); const needle = query.toLowerCase(); const results: Array<{ recordId: string; snippet: string }> = []; @@ -32,9 +33,8 @@ export function searchWorkspace( } } - for (const collection of listCollections(doc)) { - if (isAccessToken(caller) && !tokenAllowsParent(caller, collection.id)) continue; - for (const row of listRecordsForParent(doc, collection.id)) { + function searchCollectionRows(collectionId: string, collectionDoc: typeof doc): void { + for (const row of listRecordsForParent(collectionDoc, collectionId)) { for (const value of Object.values(row.properties ?? {})) { const text = value.type === 'text' || value.type === 'select' ? value.value : ''; if (text.toLowerCase().includes(needle)) { @@ -45,6 +45,31 @@ export function searchWorkspace( } } + // Catalog-listed Collections first — resolving each one's own shard from + // the locator, since a fully-sharded Collection's own meta entry (not + // just its rows) can live in a doc other than the default one, which + // listCollections(doc) below could never see. + const catalogCollectionIds = new Set(); + for (const collectionMeta of listCatalogCollections(workspaceId)) { + catalogCollectionIds.add(collectionMeta.id); + if (isAccessToken(caller) && !tokenAllowsParent(caller, collectionMeta.id)) continue; + const shard = resolveShardForParent(workspaceId, collectionMeta.id); + const collectionDoc = shard + ? resolveWorkspaceContext({ workspaceId, shardId: shard.shardId }).doc + : doc; + searchCollectionRows(collectionMeta.id, collectionDoc); + } + + // Then any Collection written directly to the Y.Doc, bypassing the + // service layer entirely (and therefore uncataloged) — the catalog loop + // above can't see these at all, so they're only findable via the default + // doc directly, matching today's completeness for that case. + for (const collection of listCollections(doc)) { + if (catalogCollectionIds.has(collection.id)) continue; + if (isAccessToken(caller) && !tokenAllowsParent(caller, collection.id)) continue; + searchCollectionRows(collection.id, doc); + } + logAudit({ actor, action: 'search_workspace', diff --git a/src/lib/services/services.test.ts b/src/lib/services/services.test.ts index bc0edc1..2cfbc97 100644 --- a/src/lib/services/services.test.ts +++ b/src/lib/services/services.test.ts @@ -30,12 +30,16 @@ import { createDocument as crdtCreateDocument, createCollection as crdtCreateCollection, getDocument as crdtGetDocument, - getCollection as crdtGetCollection + getCollection as crdtGetCollection, + getRecord as crdtGetRecord } from '$lib/data/records'; import { listCatalogCollections, listCatalogDocuments, - RecordIdConflictError + RecordIdConflictError, + reserveCollectionLocator, + recordCatalogCollectionCreated, + resolveShardForRecord } from '$lib/server/catalog'; import type { ActorId } from '$lib/data/types'; @@ -854,3 +858,142 @@ describe('service layer: catalog stays in sync with Y.Doc document/collection mu expect(crdtGetCollection(doc, direct.id)?.title).toBe('Written Directly To The Y.Doc'); }); }); + +describe('service layer: resolves a genuinely separate Collection shard (#120)', () => { + const OTHER_SHARD = 'other-shard'; + let nextId = 0; + + // createCollection always assigns shardId 'default' (the real + // shard-assignment cutover is a separate, later step — see #120) — this + // bypasses it to construct a Collection whose catalog row names a + // genuinely different shard, proving every service function resolves it + // correctly rather than assuming the default doc. + function createSyntheticShardedCollection(): { collectionId: string; workspaceId: string } { + const { workspaceId, defaultSpaceId } = resolveWorkspaceContext(); + const collectionId = `synthetic-shard-collection-${nextId++}`; + reserveCollectionLocator(workspaceId, defaultSpaceId, collectionId, OTHER_SHARD); + recordCatalogCollectionCreated({ + workspaceId, + spaceId: defaultSpaceId, + id: collectionId, + title: 'Synthetic Sharded Table', + shardId: OTHER_SHARD + }); + const { doc: otherDoc } = resolveWorkspaceContext({ workspaceId, shardId: OTHER_SHARD }); + crdtCreateCollection(otherDoc, { + id: collectionId, + title: 'Synthetic Sharded Table', + schema: [] + }); + return { collectionId, workspaceId }; + } + + it('queryCollection reads rows from the resolved shard, not the default doc', () => { + const { collectionId, workspaceId } = createSyntheticShardedCollection(); + const { doc: otherDoc } = resolveWorkspaceContext({ workspaceId, shardId: OTHER_SHARD }); + crdtCreateRecord( + otherDoc, + { + parentId: collectionId, + properties: { name: { type: 'text', value: 'Row In Other Shard' } } + }, + human + ); + + const result = queryCollection(human, collectionId); + expect(result.collection?.title).toBe('Synthetic Sharded Table'); + expect(result.records).toHaveLength(1); + }); + + it('createRecord targeting a sharded Collection writes into that shard and reserves a row locator', () => { + const { collectionId, workspaceId } = createSyntheticShardedCollection(); + + const record = createRecord(human, { + parentId: collectionId, + properties: { name: { type: 'text', value: 'New Row' } } + }); + + expect(resolveShardForRecord(workspaceId, record.id)).toEqual({ shardId: OTHER_SHARD }); + const { doc: otherDoc } = resolveWorkspaceContext({ workspaceId, shardId: OTHER_SHARD }); + expect(crdtGetRecord(otherDoc, record.id)?.properties?.name).toEqual({ + type: 'text', + value: 'New Row' + }); + }); + + it('writeRecord updates content in the resolved shard', () => { + const { collectionId, workspaceId } = createSyntheticShardedCollection(); + const record = createRecord(human, { parentId: collectionId, properties: {} }); + + writeRecord(human, record.id, { properties: { status: { type: 'text', value: 'Done' } } }); + + const { doc: otherDoc } = resolveWorkspaceContext({ workspaceId, shardId: OTHER_SHARD }); + expect(crdtGetRecord(otherDoc, record.id)?.properties?.status).toEqual({ + type: 'text', + value: 'Done' + }); + }); + + it('getRecord reads from the resolved shard', () => { + const { collectionId } = createSyntheticShardedCollection(); + const record = createRecord(human, { + parentId: collectionId, + properties: { a: { type: 'text', value: '1' } } + }); + + expect(getRecord(human, record.id)?.properties?.a).toEqual({ type: 'text', value: '1' }); + }); + + it('deleteRecord removes it from the resolved shard and releases its row locator', () => { + const { collectionId, workspaceId } = createSyntheticShardedCollection(); + const record = createRecord(human, { parentId: collectionId, properties: {} }); + + deleteRecord(human, record.id); + + expect(resolveShardForRecord(workspaceId, record.id)).toBeUndefined(); + const { doc: otherDoc } = resolveWorkspaceContext({ workspaceId, shardId: OTHER_SHARD }); + expect(crdtGetRecord(otherDoc, record.id)).toBeUndefined(); + }); + + it('holdRecords/releaseRecords (token caller) operate against the resolved shard Awareness, never the default one', () => { + const { collectionId, workspaceId } = createSyntheticShardedCollection(); + const record = createRecord(human, { parentId: collectionId, properties: {} }); + + const { record: tokenRecord } = createToken({ + clientLabel: 'Shard Test Bot', + allowedDocumentIds: [], + allowedCollectionIds: [collectionId] + }); + + const holdResult = holdRecords(tokenRecord, [record.id]); + expect(holdResult).toEqual({ granted: [record.id], denied: [] }); + + function isHeldSomewhere(workspaceIdArg: string, shardId: string | undefined): boolean { + const { awareness } = resolveWorkspaceContext( + shardId !== undefined + ? { workspaceId: workspaceIdArg, shardId } + : { workspaceId: workspaceIdArg } + ); + return Array.from(awareness.getStates().values()).some((s) => + (s as { heldRecordIds?: string[] } | undefined)?.heldRecordIds?.includes(record.id) + ); + } + + expect(isHeldSomewhere(workspaceId, OTHER_SHARD)).toBe(true); + expect(isHeldSomewhere(workspaceId, undefined)).toBe(false); + + releaseRecords(tokenRecord, [record.id]); + expect(isHeldSomewhere(workspaceId, OTHER_SHARD)).toBe(false); + }); + + it('searchWorkspace finds content living in the resolved shard, not just the default doc', () => { + const { collectionId } = createSyntheticShardedCollection(); + createRecord(human, { + parentId: collectionId, + properties: { name: { type: 'text', value: 'Findable Needle Value' } } + }); + + const results = searchWorkspace(human, 'needle'); + expect(results.some((r) => r.snippet.includes('Needle'))).toBe(true); + }); +}); From fe5bdaa2f9bf1abb996d3d3eb1c8ccb36547d159 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Sun, 30 Aug 2026 19:25:27 +0300 Subject: [PATCH 4/4] fix: workspace-scoped Space foreign keys and cross-type id collision checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - spaces gains a (workspaceId, id) unique index; catalog_documents/ catalog_collections/record_locator's spaceId is now a composite FK on (workspaceId, spaceId) -> spaces(workspaceId, id) instead of a bare spaceId -> spaces.id reference. spaces.id was already globally unique, so nothing exploits this today, but nothing in the schema previously stopped a row from storing a workspaceId that disagreed with its referenced Space's actual workspace either. - createDocument/createCollection's existing-content check (added for the prior "direct Y.Doc write" finding) only checked the same-type map. A Collection written directly to the Y.Doc, followed by createDocument with the same id, didn't collide there — documentsMap/collectionsMap are separate Y.Maps, so it wasn't a literal overwrite, but parentKindOf checks documentsMap first, making the original Collection permanently unreachable via any parentId lookup. Both creation paths now check both maps. 653/653 tests passing (5 new). Refs #113 --- ...my_menace.sql => 0002_lovely_stranger.sql} | 8 ++- drizzle/meta/0002_snapshot.json | 31 ++++++--- drizzle/meta/_journal.json | 4 +- src/lib/server/catalog.test.ts | 65 ++++++++++++++++- src/lib/server/db/schema.ts | 69 ++++++++++++++----- src/lib/services/collections.ts | 6 +- src/lib/services/documents.ts | 9 ++- src/lib/services/services.test.ts | 32 +++++++++ 8 files changed, 187 insertions(+), 37 deletions(-) rename drizzle/{0002_stormy_menace.sql => 0002_lovely_stranger.sql} (79%) diff --git a/drizzle/0002_stormy_menace.sql b/drizzle/0002_lovely_stranger.sql similarity index 79% rename from drizzle/0002_stormy_menace.sql rename to drizzle/0002_lovely_stranger.sql index 9068804..48b9d4d 100644 --- a/drizzle/0002_stormy_menace.sql +++ b/drizzle/0002_lovely_stranger.sql @@ -7,7 +7,7 @@ CREATE TABLE `catalog_collections` ( `created_at` integer NOT NULL, `updated_at` integer NOT NULL, PRIMARY KEY(`workspace_id`, `id`), - FOREIGN KEY (`space_id`) REFERENCES `spaces`(`id`) ON UPDATE no action ON DELETE no action + FOREIGN KEY (`workspace_id`,`space_id`) REFERENCES `spaces`(`workspace_id`,`id`) ON UPDATE no action ON DELETE no action ); --> statement-breakpoint CREATE TABLE `catalog_documents` ( @@ -21,7 +21,7 @@ CREATE TABLE `catalog_documents` ( `created_at` integer NOT NULL, `updated_at` integer NOT NULL, PRIMARY KEY(`workspace_id`, `id`), - FOREIGN KEY (`space_id`) REFERENCES `spaces`(`id`) ON UPDATE no action ON DELETE no action + FOREIGN KEY (`workspace_id`,`space_id`) REFERENCES `spaces`(`workspace_id`,`id`) ON UPDATE no action ON DELETE no action ); --> statement-breakpoint CREATE TABLE `catalog_outbox` ( @@ -48,7 +48,7 @@ CREATE TABLE `record_locator` ( `space_id` text NOT NULL, `shard_id` text DEFAULT 'default' NOT NULL, `created_at` integer NOT NULL, - FOREIGN KEY (`space_id`) REFERENCES `spaces`(`id`) ON UPDATE no action ON DELETE no action + FOREIGN KEY (`workspace_id`,`space_id`) REFERENCES `spaces`(`workspace_id`,`id`) ON UPDATE no action ON DELETE no action ); --> statement-breakpoint CREATE UNIQUE INDEX `record_locator_workspace_record_unique` ON `record_locator` (`workspace_id`,`record_id`);--> statement-breakpoint @@ -58,3 +58,5 @@ CREATE TABLE `spaces` ( `name` text NOT NULL, `created_at` integer NOT NULL ); +--> statement-breakpoint +CREATE UNIQUE INDEX `spaces_workspace_id_unique` ON `spaces` (`workspace_id`,`id`); \ No newline at end of file diff --git a/drizzle/meta/0002_snapshot.json b/drizzle/meta/0002_snapshot.json index e2c36f3..28f8557 100644 --- a/drizzle/meta/0002_snapshot.json +++ b/drizzle/meta/0002_snapshot.json @@ -1,7 +1,7 @@ { "version": "6", "dialect": "sqlite", - "id": "f2e87a44-6451-4ed1-baea-48026ac2b2e8", + "id": "1ee39636-c8f7-42e4-8d5b-68aa201b6088", "prevId": "ae36812a-00d1-455c-acae-ba1fde05dbb1", "tables": { "access_tokens": { @@ -165,14 +165,16 @@ }, "indexes": {}, "foreignKeys": { - "catalog_collections_space_id_spaces_id_fk": { - "name": "catalog_collections_space_id_spaces_id_fk", + "catalog_collections_workspace_id_space_id_spaces_workspace_id_id_fk": { + "name": "catalog_collections_workspace_id_space_id_spaces_workspace_id_id_fk", "tableFrom": "catalog_collections", "tableTo": "spaces", "columnsFrom": [ + "workspace_id", "space_id" ], "columnsTo": [ + "workspace_id", "id" ], "onDelete": "no action", @@ -262,14 +264,16 @@ }, "indexes": {}, "foreignKeys": { - "catalog_documents_space_id_spaces_id_fk": { - "name": "catalog_documents_space_id_spaces_id_fk", + "catalog_documents_workspace_id_space_id_spaces_workspace_id_id_fk": { + "name": "catalog_documents_workspace_id_space_id_spaces_workspace_id_id_fk", "tableFrom": "catalog_documents", "tableTo": "spaces", "columnsFrom": [ + "workspace_id", "space_id" ], "columnsTo": [ + "workspace_id", "id" ], "onDelete": "no action", @@ -447,14 +451,16 @@ } }, "foreignKeys": { - "record_locator_space_id_spaces_id_fk": { - "name": "record_locator_space_id_spaces_id_fk", + "record_locator_workspace_id_space_id_spaces_workspace_id_id_fk": { + "name": "record_locator_workspace_id_space_id_spaces_workspace_id_id_fk", "tableFrom": "record_locator", "tableTo": "spaces", "columnsFrom": [ + "workspace_id", "space_id" ], "columnsTo": [ + "workspace_id", "id" ], "onDelete": "no action", @@ -545,7 +551,16 @@ "autoincrement": false } }, - "indexes": {}, + "indexes": { + "spaces_workspace_id_unique": { + "name": "spaces_workspace_id_unique", + "columns": [ + "workspace_id", + "id" + ], + "isUnique": true + } + }, "foreignKeys": {}, "compositePrimaryKeys": {}, "uniqueConstraints": {}, diff --git a/drizzle/meta/_journal.json b/drizzle/meta/_journal.json index 310b2a6..49bc44d 100644 --- a/drizzle/meta/_journal.json +++ b/drizzle/meta/_journal.json @@ -19,8 +19,8 @@ { "idx": 2, "version": "6", - "when": 1788105346147, - "tag": "0002_stormy_menace", + "when": 1788106972389, + "tag": "0002_lovely_stranger", "breakpoints": true } ] diff --git a/src/lib/server/catalog.test.ts b/src/lib/server/catalog.test.ts index 9792929..8d10c78 100644 --- a/src/lib/server/catalog.test.ts +++ b/src/lib/server/catalog.test.ts @@ -2,7 +2,13 @@ import { describe, expect, it } from 'vitest'; import * as Y from 'yjs'; import { eq } from 'drizzle-orm'; import { getDb } from './store'; -import { catalogDocuments, catalogOutbox, catalogRevisions, spaces } from './db/schema'; +import { + catalogDocuments, + catalogOutbox, + catalogRevisions, + recordLocator, + spaces +} from './db/schema'; import { createDocument as crdtCreateDocument, createCollection as crdtCreateCollection @@ -331,3 +337,60 @@ describe('catalog: two workspaces reusing the same record id stay isolated', () ).toBe('Workspace D Table'); }); }); + +describe('catalog: spaceId is workspace-scoped, not just globally unique', () => { + it('rejects a catalog_documents row whose workspaceId disagrees with its spaceId’s real workspace', () => { + const { defaultSpaceId } = bootstrap(); + expect(() => + getDb() + .insert(catalogDocuments) + .values({ + id: 'mismatched-doc', + workspaceId: 'a-different-workspace', // defaultSpaceId belongs to WS ('default'), not this one + spaceId: defaultSpaceId, + shardId: SHARD, + title: 'Should Be Rejected', + order: 'a0', + createdAt: Date.now(), + updatedAt: Date.now() + }) + .run() + ).toThrow(/FOREIGN KEY constraint failed/); + }); + + it('rejects a record_locator row whose workspaceId disagrees with its spaceId’s real workspace', () => { + const { defaultSpaceId } = bootstrap(); + expect(() => + getDb() + .insert(recordLocator) + .values({ + workspaceId: 'a-different-workspace', + recordId: 'mismatched-record', + kind: 'document', + spaceId: defaultSpaceId, + shardId: SHARD, + createdAt: Date.now() + }) + .run() + ).toThrow(/FOREIGN KEY constraint failed/); + }); + + it('accepts a row whose workspaceId correctly matches its spaceId’s real workspace', () => { + const { defaultSpaceId } = bootstrap(); + expect(() => + getDb() + .insert(catalogDocuments) + .values({ + id: 'matched-doc', + workspaceId: WS, + spaceId: defaultSpaceId, + shardId: SHARD, + title: 'Correctly Matched', + order: 'a0', + createdAt: Date.now(), + updatedAt: Date.now() + }) + .run() + ).not.toThrow(); + }); +}); diff --git a/src/lib/server/db/schema.ts b/src/lib/server/db/schema.ts index 78d3cae..e5bd637 100644 --- a/src/lib/server/db/schema.ts +++ b/src/lib/server/db/schema.ts @@ -1,4 +1,12 @@ -import { blob, integer, primaryKey, sqliteTable, text, uniqueIndex } from 'drizzle-orm/sqlite-core'; +import { + blob, + foreignKey, + integer, + primaryKey, + sqliteTable, + text, + uniqueIndex +} from 'drizzle-orm/sqlite-core'; import type { ActorId } from '$lib/data/types'; export const snapshots = sqliteTable('snapshots', { @@ -42,12 +50,19 @@ export const accessTokens = sqliteTable('access_tokens', { // today); the column exists now so Phase B's real per-Document/per-Collection // shard split is a query-scoping change, not another migration. -export const spaces = sqliteTable('spaces', { - id: text('id').primaryKey(), - workspaceId: text('workspace_id').notNull().default('default'), - name: text('name').notNull(), - createdAt: integer('created_at').notNull() -}); +export const spaces = sqliteTable( + 'spaces', + { + id: text('id').primaryKey(), + workspaceId: text('workspace_id').notNull().default('default'), + name: text('name').notNull(), + createdAt: integer('created_at').notNull() + }, + // (workspaceId, id) is already implied unique by id's own global PK, but + // SQLite still requires an explicit unique constraint on exactly this + // column tuple to be the target of a composite foreign key below. + (t) => [uniqueIndex('spaces_workspace_id_unique').on(t.workspaceId, t.id)] +); // Primary key is (workspaceId, id), not bare id: record_locator scopes // uniqueness the same way (a recordId is only unique *within* a workspace), @@ -59,9 +74,7 @@ export const catalogDocuments = sqliteTable( { id: text('id').notNull(), // == the Y.Doc DocumentMeta.id it mirrors workspaceId: text('workspace_id').notNull().default('default'), - spaceId: text('space_id') - .notNull() - .references(() => spaces.id), + spaceId: text('space_id').notNull(), shardId: text('shard_id').notNull().default('default'), title: text('title').notNull(), // Deliberately NOT a foreign key: a Document can be created by a client @@ -77,7 +90,17 @@ export const catalogDocuments = sqliteTable( createdAt: integer('created_at').notNull(), updatedAt: integer('updated_at').notNull() }, - (t) => [primaryKey({ columns: [t.workspaceId, t.id] })] + (t) => [ + primaryKey({ columns: [t.workspaceId, t.id] }), + // Composite, not a plain spaceId -> spaces.id reference: a bare + // reference would let this row's workspaceId disagree with the + // referenced Space's own workspaceId (spaces.id alone is globally + // unique, so it can't catch that mismatch on its own). + foreignKey({ + columns: [t.workspaceId, t.spaceId], + foreignColumns: [spaces.workspaceId, spaces.id] + }) + ] ); export const catalogCollections = sqliteTable( @@ -85,9 +108,7 @@ export const catalogCollections = sqliteTable( { id: text('id').notNull(), // == the Y.Doc CollectionMeta.id it mirrors workspaceId: text('workspace_id').notNull().default('default'), - spaceId: text('space_id') - .notNull() - .references(() => spaces.id), + spaceId: text('space_id').notNull(), shardId: text('shard_id').notNull().default('default'), title: text('title').notNull(), // No parent/order (Collections are flat) and no schema mirror — schema @@ -95,7 +116,13 @@ export const catalogCollections = sqliteTable( createdAt: integer('created_at').notNull(), updatedAt: integer('updated_at').notNull() }, - (t) => [primaryKey({ columns: [t.workspaceId, t.id] })] + (t) => [ + primaryKey({ columns: [t.workspaceId, t.id] }), + foreignKey({ + columns: [t.workspaceId, t.spaceId], + foreignColumns: [spaces.workspaceId, spaces.id] + }) + ] ); // The workspace-wide (workspace_id, record_id) locator required by §3.1: the @@ -108,13 +135,17 @@ export const recordLocator = sqliteTable( workspaceId: text('workspace_id').notNull().default('default'), recordId: text('record_id').notNull(), kind: text('kind').notNull().$type<'document' | 'collection'>(), - spaceId: text('space_id') - .notNull() - .references(() => spaces.id), + spaceId: text('space_id').notNull(), shardId: text('shard_id').notNull().default('default'), createdAt: integer('created_at').notNull() }, - (t) => [uniqueIndex('record_locator_workspace_record_unique').on(t.workspaceId, t.recordId)] + (t) => [ + uniqueIndex('record_locator_workspace_record_unique').on(t.workspaceId, t.recordId), + foreignKey({ + columns: [t.workspaceId, t.spaceId], + foreignColumns: [spaces.workspaceId, spaces.id] + }) + ] ); export const catalogRevisions = sqliteTable('catalog_revisions', { diff --git a/src/lib/services/collections.ts b/src/lib/services/collections.ts index 8ed7e84..99b9b1c 100644 --- a/src/lib/services/collections.ts +++ b/src/lib/services/collections.ts @@ -3,6 +3,7 @@ import { createCollection as crdtCreateCollection, deleteCollection as crdtDeleteCollection, getCollection as crdtGetCollection, + getDocument as crdtGetDocument, listCollections as crdtListCollections, listRecordsForParent as crdtListRecordsForParent, updateCollectionTitle as crdtUpdateCollectionTitle @@ -41,9 +42,10 @@ export function createCollection( const id = input.id ?? nanoid(); // See documents.ts's createDocument for why this also checks the live // Y.Doc, not just the catalog locator: a caller-supplied id could collide - // with a Collection created by a client writing directly to the Y.Doc, + // with content created by a client writing directly to the Y.Doc, // bypassing the service layer (and therefore the locator) entirely. - if (crdtGetCollection(doc, id)) { + // Checked against both maps — see createDocument's comment for why. + if (crdtGetCollection(doc, id) || crdtGetDocument(doc, id)) { throw new RecordIdConflictError(id); } reserveCollectionLocator(workspaceId, defaultSpaceId, id, shardId); diff --git a/src/lib/services/documents.ts b/src/lib/services/documents.ts index 3ab972e..edc85df 100644 --- a/src/lib/services/documents.ts +++ b/src/lib/services/documents.ts @@ -3,6 +3,7 @@ import { createDocument as crdtCreateDocument, createRecord as crdtCreateRecord, deleteDocument as crdtDeleteDocument, + getCollection as crdtGetCollection, getDocument as crdtGetDocument, listDocuments as crdtListDocuments, listRecordsForParent as crdtListRecordsForParent, @@ -55,9 +56,13 @@ export function createDocument(caller: CallerIdentity, input: CreateDocumentInpu // can't catch a collision with content a client wrote directly to the // Y.Doc (bypassing the service layer, and therefore the locator) — a // caller-supplied id is checked against the live Y.Doc too, since - // crdtCreateDocument would otherwise silently overwrite it. + // crdtCreateDocument would otherwise silently overwrite it. Checked + // against *both* maps: an id colliding with an existing Collection + // wouldn't overwrite it (documents/collections are separate Y.Maps), but + // would leave it permanently unreachable via parentKindOf, which checks + // the documents map first. const id = input.id ?? nanoid(); - if (crdtGetDocument(doc, id)) { + if (crdtGetDocument(doc, id) || crdtGetCollection(doc, id)) { throw new RecordIdConflictError(id); } reserveDocumentLocator(workspaceId, defaultSpaceId, id, shardId); diff --git a/src/lib/services/services.test.ts b/src/lib/services/services.test.ts index bc0edc1..b4b7886 100644 --- a/src/lib/services/services.test.ts +++ b/src/lib/services/services.test.ts @@ -853,4 +853,36 @@ describe('service layer: catalog stays in sync with Y.Doc document/collection mu ).toThrow(RecordIdConflictError); expect(crdtGetCollection(doc, direct.id)?.title).toBe('Written Directly To The Y.Doc'); }); + + it('rejects createDocument when the id already names a Collection (cross-type collision, direct Y.Doc write)', () => { + const { doc } = resolveWorkspaceContext(); + const directCollection = crdtCreateCollection(doc, { + title: 'A Collection, Written Directly', + schema: [] + }); + + expect(() => + createDocument(human, { id: directCollection.id, title: 'Cross-Type Attempt' }) + ).toThrow(RecordIdConflictError); + // The Collection must remain intact and still reachable — not silently + // shadowed by a same-id Document entry (documentsMap/collectionsMap are + // separate Y.Maps, so a same-id Document wouldn't overwrite it, but + // parentKindOf checks the documents map first, making the Collection + // permanently unreachable via any parentId lookup once both exist). + expect(crdtGetCollection(doc, directCollection.id)?.title).toBe( + 'A Collection, Written Directly' + ); + expect(crdtGetDocument(doc, directCollection.id)).toBeUndefined(); + }); + + it('rejects createCollection when the id already names a Document (cross-type collision, direct Y.Doc write)', () => { + const { doc } = resolveWorkspaceContext(); + const directDocument = crdtCreateDocument(doc, { title: 'A Document, Written Directly' }); + + expect(() => + createCollection(human, { id: directDocument.id, title: 'Cross-Type Attempt', schema: [] }) + ).toThrow(RecordIdConflictError); + expect(crdtGetDocument(doc, directDocument.id)?.title).toBe('A Document, Written Directly'); + expect(crdtGetCollection(doc, directDocument.id)).toBeUndefined(); + }); });