Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/specifications/service-layer-manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ The manifest guarantees every method has a recorded _intent_. It cannot, by itse
1. **Generate the MCP tool table from the manifest, don't hand-write it.** For every `mcp: true` entry, a thin generic wrapper (`(input) => serviceModules[module][method](actor, input)`) registers the tool — there is no second hand-written call site to drift, because there's only one. Reserve hand-written MCP tool definitions for the rare method that needs genuinely custom input shaping beyond what the service function's own parameter type already describes.
2. **A wiring-check test for the UI side.** SvelteKit's file-based routing means routes/actions can't be generated the same way (each often has form-specific validation, redirects, or multi-step flows). Instead, add a Tier A test (see [`e2e-testing.md`](./e2e-testing.md) §2) that walks `serviceSurfaces`, and for every `ui: true` entry, drives the real route/action through the test harness and asserts the underlying service function actually ran (e.g. by asserting its observable effect — the audit entry, the persisted state — the same way other Tier A tests already assert protocol-boundary correctness). This is the same "second, independent call observes the real effect" pattern already established for the MCP side; applying it to the manifest costs one parametrized test, not N bespoke ones.

Static typing alone gets you "nothing was forgotten from the list." It cannot get you "the thing on the list is wired correctly" — that residual has to be a test, and the manifest is what makes that test parametrized and complete instead of another hand-maintained list.
Static typing alone gets you "nothing was forgotten from the list." It cannot get you "the thing on the list is wired correctly" — that residual has to be a test, and the manifest is what makes that test parametrized and complete instead of another hand-maintained list. The implemented manifest also owns typed `mcpAdapterBindings` and `uiAdapterBindings`: tests prove both maps match the declared surfaces exactly (no missing adapter and no undeclared adapter).

## 4. What this fixes, concretely

Expand All @@ -68,7 +68,7 @@ Additive, and sequenced strictly after `service-layer.md`'s M1 gives it somethin
1. Once `src/lib/services/documents.ts` exists (service-layer spec §5 step 1), add `src/lib/services/manifest.ts` covering just that module — `ServiceMethod` and `serviceSurfaces` don't need every aggregate populated on day one, only the ones that exist yet.
2. Regenerate the MCP tool registrations for `documents.*` from the manifest (§3.1) as part of the same work that points `create_document`'s handler at the new service function (service-layer spec §5 step 2) — this is the natural moment, since that handler is already being rewritten.
3. Add the one parametrized UI wiring-check test (§3.2) to the Tier A suite once `tests/e2e/harness.ts` exists (`e2e-testing.md` §3 / `phase-2-plan.md` M2).
4. Extend `serviceModules` / `serviceSurfaces` to `records`, `holds`, `collections`, `search` opportunistically, in step with `service-layer.md` §5 step 4's own opportunistic migration — the two migrations track each other module-by-module.
4. Extend `serviceModules` / `serviceSurfaces` to `records`, `holds`, `collections`, `search`, `spaces`, `tokens`, and audit history in step with the service migration — every exported use case has an explicit surface decision.

## 6. Testing implications

Expand Down
2 changes: 1 addition & 1 deletion docs/specifications/service-layer.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ src/lib/services/
revokeToken(actor, tokenHash) → void
```

`tokens.ts` and `spaces.ts` are UI-only (`settings/tokens`, `/api/spaces`) — no MCP tool exposes minting or revoking a token or creating a Space, so neither is registered in `services/manifest.ts`'s MCP/UI wiring table (that table's job is enforcing MCP-tool ↔ UI-surface parity, which doesn't apply to a use case with no MCP side at all; see `manifest.ts`'s existing `spaces` precedent). `tokens.ts#createToken` still validates `allowedSpaceIds` against the workspace's real Spaces before persisting (#188) — a crafted request could otherwise grant a token access to a Space id that merely happens to exist somewhere else, since Space membership alone later authorizes access (`tokenAllowsParent`). Reading the token list (`listTokens`) stays a plain, policy-free lookup called directly from the route, same precedent as `queryAuditLog`.
`tokens.ts` and `spaces.ts` are UI-only (`settings/tokens`, `/api/spaces`) — no MCP tool exposes minting or revoking a token or creating a Space. They are nevertheless registered in `services/manifest.ts` with `mcp: false, ui: true`, alongside token listing and audit-history listing, so adapter ownership is enforced without accidentally exposing them over MCP. `tokens.ts#createToken` still validates `allowedSpaceIds` against the workspace's real Spaces before persisting (#188) — a crafted request could otherwise grant a token access to a Space id that merely happens to exist somewhere else, since Space membership alone later authorizes access (`tokenAllowsParent`).

Each function's first parameter is whatever identifies the caller for permission purposes — an `AccessToken` for MCP-originated calls, the fixed `CURRENT_USER` `ActorId` for Phase 0/1 UI calls (see `data-model.md` §1's `ActorId` union; this doesn't need to change). Where MCP and UI calls to the "same" use case need different permission rules (e.g. UI writes are currently unscoped, single-tenant; MCP writes are token-scoped), the service function is the one place that branches on that — not duplicated per adapter.

Expand Down
49 changes: 27 additions & 22 deletions src/lib/data/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ export type ActorId =
| { kind: 'agent'; agentId: string; name: string }
| { kind: 'human-via-client'; userId: string; client: string }; // "Brylie · via Claude Desktop"

export type PropertyType = 'text' | 'number' | 'date' | 'select' | 'checkbox' | 'relation';
/** Canonical runtime discriminator list shared by data validation and adapters. */
export const propertyTypes = ['text', 'number', 'date', 'select', 'checkbox', 'relation'] as const;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
export type PropertyType = (typeof propertyTypes)[number];

export type PropertyValue =
| { type: 'text'; value: string }
Expand All @@ -25,27 +27,30 @@ export interface PropertyDefinition {
targetCollectionId?: string; // for 'relation' — which Collection its record-id values point into
}

export type BlockType =
| 'paragraph'
| 'heading_1'
| 'heading_2'
| 'heading_3'
| 'heading_4'
| 'bulleted_list_item'
| 'numbered_list_item'
| 'to_do'
| 'quote'
| 'divider'
| 'callout'
| 'toggle'
| 'table'
| 'code'
| 'table_of_contents'
| 'synced_block'
| 'page_link'
| 'embed'
| 'collection_view' // embeds a Table/Board/Calendar view of a Collection inline in a Document — see collection-views.md
| 'child_pages'; // live listing of a Document's sub-pages (Confluence-style page tree) — issue #43
/** Canonical runtime discriminator list for Document blocks. */
export const blockTypes = [
'paragraph',
'heading_1',
'heading_2',
'heading_3',
'heading_4',
'bulleted_list_item',
'numbered_list_item',
'to_do',
'quote',
'divider',
'callout',
'toggle',
'table',
'code',
'table_of_contents',
'synced_block',
'page_link',
'embed',
'collection_view',
'child_pages'
] as const;
export type BlockType = (typeof blockTypes)[number];

// "View" here means a Collection/database view (Table/Board/Calendar — a
// rendering + configuration over a Collection's records), never an MVC-style
Expand Down
46 changes: 17 additions & 29 deletions src/lib/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,29 @@ import { verifyToken, type AccessToken } from './tokens';
import {
serviceModules,
serviceSurfaces,
mcpAdapterBindings,
type ServiceMethod,
PermissionDeniedError,
HoldRequiredError
} from '$lib/services';
import type { BlockType, EmbeddedViewConfig } from '$lib/data/types';
import {
blockTypes,
propertyTypes,
type BlockType,
type EmbeddedViewConfig
} from '$lib/data/types';
import { resolvePrimaryField } from '$lib/data/records';

const propertyValueSchema = z.discriminatedUnion('type', [
z.object({ type: z.literal('text'), value: z.string() }),
z.object({ type: z.literal('number'), value: z.number() }),
z.object({ type: z.literal('date'), value: z.string() }),
z.object({ type: z.literal('select'), value: z.string() }),
z.object({ type: z.literal('checkbox'), value: z.boolean() }),
z.object({ type: z.literal('relation'), value: z.array(z.string()) })
z.object({ type: z.literal(propertyTypes[0]), value: z.string() }),
z.object({ type: z.literal(propertyTypes[1]), value: z.number() }),
z.object({ type: z.literal(propertyTypes[2]), value: z.string() }),
z.object({ type: z.literal(propertyTypes[3]), value: z.string() }),
z.object({ type: z.literal(propertyTypes[4]), value: z.boolean() }),
z.object({ type: z.literal(propertyTypes[5]), value: z.array(z.string()) })
]);

const blockTypeSchema = z.enum([
'paragraph',
'heading_1',
'heading_2',
'heading_3',
'heading_4',
'bulleted_list_item',
'numbered_list_item',
'to_do',
'quote',
'divider',
'callout',
'toggle',
'table',
'code',
'table_of_contents',
'synced_block',
'page_link',
'embed',
'collection_view',
'child_pages'
]);
const blockTypeSchema = z.enum(blockTypes);

const childPagesDepthSchema = z.union([z.number().int().positive(), z.literal('unlimited')]);

Expand Down Expand Up @@ -113,6 +98,9 @@ function registerFromManifest<Args extends z.ZodRawShape>(
`Service method "${method}" is not declared as an MCP tool with valid name and description in manifest`
);
}
if (mcpAdapterBindings[method as keyof typeof mcpAdapterBindings] !== surface.mcpToolName) {
throw new Error(`MCP adapter binding for service method "${method}" is missing or mismatched`);
}
const register = server.registerTool.bind(server) as (
name: string,
config: { description?: string; inputSchema: Args },
Expand Down
141 changes: 13 additions & 128 deletions src/lib/mcp/tokens.ts
Original file line number Diff line number Diff line change
@@ -1,130 +1,15 @@
import { createHash, randomBytes } from 'node:crypto';
import { desc, eq } from 'drizzle-orm';
import { getDb } from '$lib/server/store.js';
import { accessTokens } from '$lib/server/db/schema.js';

export interface AccessToken {
tokenHash: string;
clientLabel: string;
allowedDocumentIds: string[];
allowedCollectionIds: string[];
allowedSpaceIds: string[];
createdAt: number;
revokedAt?: number;
}

function rowToToken(row: typeof accessTokens.$inferSelect): AccessToken {
return {
tokenHash: row.tokenHash,
clientLabel: row.clientLabel,
allowedDocumentIds: row.allowedDocumentIds,
allowedCollectionIds: row.allowedCollectionIds,
allowedSpaceIds: row.allowedSpaceIds,
createdAt: row.createdAt,
revokedAt: row.revokedAt ?? undefined
};
}

/** Derives the stored lookup/comparison key for a bearer token — only this hash is ever persisted, never the raw token. */
export function hashToken(token: string): string {
return createHash('sha256').update(token).digest('hex');
}

/** Returns the raw bearer token once — only its hash is ever stored. */
export function createToken(input: {
clientLabel: string;
allowedDocumentIds: string[];
allowedCollectionIds: string[];
allowedSpaceIds?: string[];
}): { token: string; record: AccessToken } {
const token = `as_${randomBytes(24).toString('base64url')}`;
const record: AccessToken = {
tokenHash: hashToken(token),
clientLabel: input.clientLabel,
allowedDocumentIds: input.allowedDocumentIds,
allowedCollectionIds: input.allowedCollectionIds,
allowedSpaceIds: input.allowedSpaceIds ?? [],
createdAt: Date.now()
};

getDb().insert(accessTokens).values(record).run();

return { token, record };
}

/** Verifies a raw bearer token and returns its (non-revoked) record, or null. */
export function verifyToken(token: string): AccessToken | null {
const row = getDb()
.select()
.from(accessTokens)
.where(eq(accessTokens.tokenHash, hashToken(token)))
.get();
if (!row || row.revokedAt) return null;
return rowToToken(row);
}

/** Lists all access tokens (including revoked ones), newest first, for the token-management UI. */
export function listTokens(): AccessToken[] {
const rows = getDb().select().from(accessTokens).orderBy(desc(accessTokens.createdAt)).all();
return rows.map(rowToToken);
}

/** A team member can revoke their own client's connection at any time — no admin action required (PRD). */
export function revokeToken(tokenHash: string): void {
getDb()
.update(accessTokens)
.set({ revokedAt: Date.now() })
.where(eq(accessTokens.tokenHash, tokenHash))
.run();
}

/** Persists an access grant for a newly created document to SQLite so subsequent tool calls succeed. */
export function grantDocumentAccess(tokenHash: string, documentId: string): void {
const db = getDb();
const row = db
.select({ allowedDocumentIds: accessTokens.allowedDocumentIds })
.from(accessTokens)
.where(eq(accessTokens.tokenHash, tokenHash))
.get();
if (!row) return;
if (!row.allowedDocumentIds.includes(documentId)) {
db.update(accessTokens)
.set({ allowedDocumentIds: [...row.allowedDocumentIds, documentId] })
.where(eq(accessTokens.tokenHash, tokenHash))
.run();
}
}

/** Persists an access grant for a newly created collection to SQLite so subsequent tool calls succeed. */
export function grantCollectionAccess(tokenHash: string, collectionId: string): void {
const db = getDb();
const row = db
.select({ allowedCollectionIds: accessTokens.allowedCollectionIds })
.from(accessTokens)
.where(eq(accessTokens.tokenHash, tokenHash))
.get();
if (!row) return;
if (!row.allowedCollectionIds.includes(collectionId)) {
db.update(accessTokens)
.set({ allowedCollectionIds: [...row.allowedCollectionIds, collectionId] })
.where(eq(accessTokens.tokenHash, tokenHash))
.run();
}
}

/**
* True when `token` may access `parentId` — either because it's directly
* allowlisted (per-Document/per-Collection grant), or because `spaceId` (the
* record's own catalog Space, when the caller has it — see
* services/permissions.ts's resolveParentWorkspaceContext) is one of the
* token's Space-level grants (#6). Resolved live against the token's current
* `allowedSpaceIds`, not backfilled onto individual records, so a Space
* grant automatically covers content created in that Space later.
* Compatibility exports for MCP transport callers. Token persistence belongs
* to the neutral server store so application services do not depend on MCP.
*/
export function tokenAllowsParent(token: AccessToken, parentId: string, spaceId?: string): boolean {
return (
token.allowedDocumentIds.includes(parentId) ||
token.allowedCollectionIds.includes(parentId) ||
(spaceId !== undefined && token.allowedSpaceIds.includes(spaceId))
);
}
export {
createToken,
grantCollectionAccess,
grantDocumentAccess,
hashToken,
listTokens,
revokeToken,
tokenAllowsParent,
verifyToken,
type AccessToken
} from '$lib/server/token-store';
Loading
Loading