From 9e21eb5d103ec4f1372986f3be1d17f6ef40feb2 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 28 Aug 2026 13:07:40 +0200 Subject: [PATCH] feat(mcp): note colours, note order, a note website, and category CRUD A day note could not be coloured or positioned, so the service default of 9999 pinned every note an assistant wrote to the bottom of the timeline. A collab note could not carry the website the card renders. Categories could be assigned by bulk_update_places but never created: the whole file was one read tool and a resource. The three new tools are admin-only, matching the REST routes exactly. --- server/src/nest/categories/categories.mcp.ts | 78 ++++- .../src/nest/categories/categories.module.ts | 5 + server/src/nest/collab/collab.mcp.ts | 18 +- server/src/nest/day-notes/day-notes.mcp.ts | 30 +- .../tests/unit/mcp/tools-categories.test.ts | 275 +++++++++++++++++- server/tests/unit/mcp/tools-notes.test.ts | 185 ++++++++++++ 6 files changed, 570 insertions(+), 21 deletions(-) diff --git a/server/src/nest/categories/categories.mcp.ts b/server/src/nest/categories/categories.mcp.ts index e9aa279b0..e87162a17 100644 --- a/server/src/nest/categories/categories.mcp.ts +++ b/server/src/nest/categories/categories.mcp.ts @@ -1,7 +1,14 @@ import { McpController, Tool, Resource, type McpContext, - TOOL_ANNOTATIONS_READONLY, ok, + TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, + TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, + demoDenied, errorResult, ok, } from '../../nest-mcp'; +import { z } from 'zod'; +import { createCategoryRequestSchema, updateCategoryRequestSchema } from '@trek/shared'; +import { AuthService } from '../auth/auth.service'; +import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; +import { adminRequired } from '../../mcp/tools/_shared'; import { CategoriesService } from './categories.service'; /** @@ -12,10 +19,21 @@ import { CategoriesService } from './categories.service'; * src/mcp/resources.ts (first production @Resource; intentionally ungated — * the legacy registration was unconditional, safe for any authenticated * session — and the payload reproduces the legacy jsonContent shape verbatim). + * + * The three write tools mirror POST/PUT/DELETE /api/categories, which sit + * behind JwtAuthGuard + AdminGuard: the palette is instance-wide, not + * trip-scoped, so there is no trip to check access against and the admin role + * is the whole gate. Bodies reuse the same @trek/shared contracts the + * controller's DTOs wrap, so `color` stays hex-only on both surfaces (it is + * interpolated into a style="…" attribute of hand-built marker HTML). */ @McpController() export class CategoriesMcp { - constructor(private readonly categories: CategoriesService) {} + constructor( + private readonly categories: CategoriesService, + private readonly auth: AuthService, + private readonly guards: McpToolGuardsService, + ) {} @Tool({ name: 'list_categories', @@ -29,6 +47,62 @@ export class CategoriesMcp { return ok({ categories }); } + @Tool({ + name: 'create_category', + description: 'Add a new place category to the instance-wide palette. Admin only. Prefer an existing category from list_categories: this mints one every trip on the instance will see, so only reach for it when nothing in the palette fits.', + inputSchema: { + name: createCategoryRequestSchema.shape.name.describe('Category label, e.g. "Street food"'), + color: createCategoryRequestSchema.shape.color.describe('Hex colour for the map marker (defaults to #6366f1)'), + icon: createCategoryRequestSchema.shape.icon.describe('Emoji shown on the marker (defaults to 📍)'), + }, + annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, + access: { group: 'places', mode: 'write' }, + }) + async createCategory({ name, color, icon }: { name: string; color?: string; icon?: string }, ctx: McpContext) { + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + // The palette is instance-wide; the REST route restricts management to admins. Match it. + if (!this.guards.isAdminUser(ctx.userId)) return adminRequired(); + const category = this.categories.create(ctx.userId, name, color, icon); + return ok({ category }); + } + + @Tool({ + name: 'update_category', + description: 'Rename an existing place category or change its colour or icon. Admin only. Every place already carrying the category follows the change, so use this to fix a palette entry rather than to reclassify places.', + inputSchema: { + categoryId: z.number().int().positive().describe('Category ID from list_categories'), + name: updateCategoryRequestSchema.shape.name, + color: updateCategoryRequestSchema.shape.color.describe('Hex colour for the map marker'), + icon: updateCategoryRequestSchema.shape.icon.describe('Emoji shown on the marker'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'places', mode: 'write' }, + }) + async updateCategory({ categoryId, name, color, icon }: { categoryId: number; name?: string; color?: string; icon?: string }, ctx: McpContext) { + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (!this.guards.isAdminUser(ctx.userId)) return adminRequired(); + if (!this.categories.getById(categoryId)) return errorResult('Category not found'); + const category = this.categories.update(categoryId, name, color, icon); + return ok({ category }); + } + + @Tool({ + name: 'delete_category', + description: 'Remove a place category from the instance-wide palette. Admin only. Places keep their data but lose the category, across every trip on the instance. Use update_category when the entry only needs fixing.', + inputSchema: { + categoryId: z.number().int().positive().describe('Category ID from list_categories'), + }, + annotations: TOOL_ANNOTATIONS_DELETE, + access: { group: 'places', mode: 'write' }, + }) + async deleteCategory({ categoryId }: { categoryId: number }, ctx: McpContext) { + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (!this.guards.isAdminUser(ctx.userId)) return adminRequired(); + if (!this.categories.getById(categoryId)) return errorResult('Category not found'); + this.categories.remove(categoryId); + return ok({ success: true }); + } + @Resource({ name: 'categories', uri: 'trek://categories', diff --git a/server/src/nest/categories/categories.module.ts b/server/src/nest/categories/categories.module.ts index b43c5d516..8a11459b9 100644 --- a/server/src/nest/categories/categories.module.ts +++ b/server/src/nest/categories/categories.module.ts @@ -1,4 +1,6 @@ import { Module } from '@nestjs/common'; +import { AuthModule } from '../auth/auth.module'; +import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; import { CategoriesController } from './categories.controller'; import { CategoriesMcp } from './categories.mcp'; import { CategoriesService } from './categories.service'; @@ -6,6 +8,9 @@ import { CategoriesRpc } from './categories.rpc'; /** Categories domain (L4 leaf module). Registered in AppModule. */ @Module({ + // CategoriesMcp gates its three admin tools on AuthService, and McpSharedModule + // is deliberately not @Global, so both are imported rather than inherited. + imports: [AuthModule, McpSharedModule], controllers: [CategoriesController], providers: [CategoriesService, CategoriesMcp, CategoriesRpc], // For in-container consumers (CategoriesRpc). diff --git a/server/src/nest/collab/collab.mcp.ts b/server/src/nest/collab/collab.mcp.ts index 0c4778f2a..c344f1b9a 100644 --- a/server/src/nest/collab/collab.mcp.ts +++ b/server/src/nest/collab/collab.mcp.ts @@ -57,7 +57,9 @@ function jsonContent(uri: string, data: unknown) { * read/write access markers (the legacy `if (R)` / `if (W)` checks, resolved * by trekMcpAccessPolicy). The list tools check only trip access (as legacy); * vote_collab_poll gained the demo-user gate the legacy registrar was missing, - * matching every other collab write tool. + * matching every other collab write tool. The two note write tools have since + * gained `website`, which the REST route has always accepted and the note card + * renders as a link preview. */ @McpController() export class CollabMcp { @@ -79,6 +81,7 @@ export class CollabMcp { content: z.string().max(10000).optional(), category: z.string().max(100).optional().describe('Note category (e.g. "Ideas", "To-do", "General")'), color: z.string().regex(/^#[0-9a-fA-F]{6}$/).optional().describe('Hex color for the note card'), + website: z.string().max(500).nullable().optional().describe('Link to attach to the note; the card renders it as a preview thumbnail. Pass null to remove it'), pinned: z.boolean().optional().default(false).describe('Pin the note to the top'), }, annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, @@ -86,15 +89,15 @@ export class CollabMcp { access: { group: 'collab', mode: 'write' }, }) async createCollabNote( - { tripId, title, content, category, color, pinned }: { - tripId: number; title: string; content?: string; category?: string; color?: string; pinned?: boolean; + { tripId, title, content, category, color, website, pinned }: { + tripId: number; title: string; content?: string; category?: string; color?: string; website?: string | null; pinned?: boolean; }, ctx: McpContext, ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); - const note = this.collab.createNote(tripId, ctx.userId, { title, content, category, color, pinned }); + const note = this.collab.createNote(tripId, ctx.userId, { title, content, category, color, website, pinned }); this.guards.safeBroadcast(tripId, 'collab:note:created', { note }); return ok({ note }); } @@ -109,6 +112,7 @@ export class CollabMcp { content: z.string().max(10000).optional(), category: z.string().max(100).optional(), color: z.string().regex(/^#[0-9a-fA-F]{6}$/).optional().describe('Hex color for the note card'), + website: z.string().max(500).nullable().optional().describe('Link to attach to the note, or null to remove the one it has'), pinned: z.boolean().optional().describe('Pin the note to the top'), }, annotations: TOOL_ANNOTATIONS_WRITE, @@ -116,15 +120,15 @@ export class CollabMcp { access: { group: 'collab', mode: 'write' }, }) async updateCollabNote( - { tripId, noteId, title, content, category, color, pinned }: { - tripId: number; noteId: number; title?: string; content?: string; category?: string; color?: string; pinned?: boolean; + { tripId, noteId, title, content, category, color, website, pinned }: { + tripId: number; noteId: number; title?: string; content?: string; category?: string; color?: string; website?: string | null; pinned?: boolean; }, ctx: McpContext, ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied(); - const note = this.collab.updateNote(tripId, noteId, { title, content, category, color, pinned }); + const note = this.collab.updateNote(tripId, noteId, { title, content, category, color, website, pinned }); if (!note) return errorResult('Note not found.'); this.guards.safeBroadcast(tripId, 'collab:note:updated', { note }); return ok({ note }); diff --git a/server/src/nest/day-notes/day-notes.mcp.ts b/server/src/nest/day-notes/day-notes.mcp.ts index a0ba802f7..69a8d3b88 100644 --- a/server/src/nest/day-notes/day-notes.mcp.ts +++ b/server/src/nest/day-notes/day-notes.mcp.ts @@ -5,10 +5,19 @@ import { } from '../../nest-mcp'; import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { z } from 'zod'; +import { NOTE_COLORS, type NoteColor } from '@trek/shared'; import { AuthService } from '../auth/auth.service'; import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { DayNotesService } from './day-notes.service'; +/** + * The palette itself rather than the REST contract's `z.string().max(9)`. + * normalizeNoteColor() stores anything off-palette as "no colour", so a loose + * string would let a caller send #ff0000, get a success back and never learn the + * colour was dropped. An enum is also how the tool tells a model what it may pick. + */ +const noteColorSchema = z.enum(NOTE_COLORS); + function parseId(value: string | string[]): number | null { const n = Number(Array.isArray(value) ? value[0] : value); return Number.isInteger(n) && n > 0 ? n : null; @@ -23,6 +32,11 @@ function parseId(value: string | string[]): number | null { * trips write/read access markers (registerDayTools' whole-registrar * `canWrite(scopes, 'trips')` early return and the resource's canReadTrips * check, resolved by trekMcpAccessPolicy). No addon gate — day notes are core. + * + * The write tools have since gained `color` and `sort_order`, which the REST + * contract carried all along: without them an MCP note could not be coloured + * and landed at the bottom of the day's merged timeline whatever the caller + * meant. */ @McpController() export class DayNotesMcp { @@ -41,19 +55,23 @@ export class DayNotesMcp { text: z.string().min(1).max(500), time: z.string().max(250).optional().describe('Time label (e.g. "09:00" or "Morning")'), icon: z.string().max(64).optional().describe('Emoji icon for the note'), + color: noteColorSchema.nullable().optional().describe('Card colour from the note palette; null or omitted leaves the neutral card'), + sort_order: z.number().optional().describe('Position in the day, lowest first, interleaved with the places of that day. Omit to append at the bottom'), }, annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, access: { group: 'trips', mode: 'write' }, }) async createDayNote( - { tripId, dayId, text, time, icon }: { tripId: number; dayId: number; text: string; time?: string; icon?: string }, + { tripId, dayId, text, time, icon, color, sort_order }: { + tripId: number; dayId: number; text: string; time?: string; icon?: string; color?: NoteColor | null; sort_order?: number; + }, ctx: McpContext, ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.notes.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.notes.dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; - const note = this.notes.create(dayId, tripId, text, time, icon); + const note = this.notes.create(dayId, tripId, text, time, icon, sort_order, color); this.guards.safeBroadcast(tripId, 'dayNote:created', { dayId, note }); return ok({ note }); } @@ -68,12 +86,16 @@ export class DayNotesMcp { text: z.string().min(1).max(500).optional(), time: z.string().max(250).nullable().optional().describe('Time label (e.g. "09:00" or "Morning"), or null to clear'), icon: z.string().max(64).optional().describe('Emoji icon for the note'), + color: noteColorSchema.nullable().optional().describe('Card colour from the note palette, or null to go back to the neutral card'), + sort_order: z.number().optional().describe('New position in the day, lowest first, interleaved with the places of that day'), }, annotations: TOOL_ANNOTATIONS_WRITE, access: { group: 'trips', mode: 'write' }, }) async updateDayNote( - { tripId, dayId, noteId, text, time, icon }: { tripId: number; dayId: number; noteId: number; text?: string; time?: string | null; icon?: string }, + { tripId, dayId, noteId, text, time, icon, color, sort_order }: { + tripId: number; dayId: number; noteId: number; text?: string; time?: string | null; icon?: string; color?: NoteColor | null; sort_order?: number; + }, ctx: McpContext, ) { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); @@ -81,7 +103,7 @@ export class DayNotesMcp { if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const existing = this.notes.getNote(noteId, dayId, tripId); if (!existing) return { content: [{ type: 'text' as const, text: 'Note not found.' }], isError: true }; - const note = this.notes.update(noteId, existing, { text, time: time !== undefined ? time : undefined, icon }); + const note = this.notes.update(noteId, existing, { text, time: time !== undefined ? time : undefined, icon, color, sort_order }); this.guards.safeBroadcast(tripId, 'dayNote:updated', { dayId, note }); return ok({ note }); } diff --git a/server/tests/unit/mcp/tools-categories.test.ts b/server/tests/unit/mcp/tools-categories.test.ts index 95e52973f..7068b4f97 100644 --- a/server/tests/unit/mcp/tools-categories.test.ts +++ b/server/tests/unit/mcp/tools-categories.test.ts @@ -1,9 +1,10 @@ /** * Unit tests for the categories MCP surface (CategoriesMcp, DI-discovered): - * the list_categories tool and the trek://categories resource. + * the list_categories tool, the create/update/delete_category admin tools and + * the trek://categories resource. * - * Both attach via the nest-mcp registry inside registerTools, so every harness - * here keeps withTools on (the resource is NOT registered by the legacy + * They all attach via the nest-mcp registry inside registerTools, so every + * harness here keeps withTools on (the resource is NOT registered by the legacy * registerResources fan-out anymore). */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; @@ -32,11 +33,31 @@ vi.mock('../../../src/config', () => ({ updateJwtSecret: () => {}, })); +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import { Client } from '@modelcontextprotocol/sdk/client/index'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory'; import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; -import { createUser } from '../../helpers/factories'; +import { createUser, createAdmin } from '../../helpers/factories'; import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; +import { createTestRegistry } from '../../../src/nest-mcp'; +import { trekMcpAccessPolicy, trekMcpValidateAccess } from '../../../src/mcp/nest-mcp-policy'; +import { CategoriesMcp } from '../../../src/nest/categories/categories.mcp'; +import { CategoriesService } from '../../../src/nest/categories/categories.service'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import { McpToolGuardsService } from '../../../src/nest/mcp-shared/mcp-tool-guards.service'; +import { AuthService } from '../../../src/nest/auth/auth.service'; +import { BudgetService } from '../../../src/nest/budget/budget.service'; +import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; +import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; +import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; +import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; +import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; +import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; +import { AllowedFileTypesService } from '../../../src/nest/files/allowed-file-types.service'; beforeAll(() => { createTables(testDb); @@ -45,6 +66,7 @@ beforeAll(() => { beforeEach(() => { resetTestDb(testDb); + delete process.env.DEMO_MODE; }); afterAll(() => { @@ -60,6 +82,46 @@ async function withHarness( try { await fn(h); } finally { await h.cleanup(); } } +// The write tools reach the auth and guard collaborators, so they run against a +// controller wired here rather than through the shared harness. Everything +// points at this file's DB, which is what lets the admin gate and the demo gate +// be driven from the users table instead of from a stub. +const categoriesDb = new DatabaseService(testDb); +const categoriesMcp = new CategoriesMcp( + new CategoriesService(categoriesDb), + new AuthService( + categoriesDb, + new PermissionsService(categoriesDb), + new TripMembershipService(categoriesDb), + new WebauthnConfigService(categoriesDb), + new UserCleanupService(categoriesDb, new BudgetService(categoriesDb, new PermissionsService(categoriesDb), new ExchangeRatesService(), new RealtimeService())), + new MailerService(categoriesDb), + new EphemeralTokenService(), + new AllowedFileTypesService(categoriesDb), + ), + new McpToolGuardsService(categoriesDb, new PermissionsService(categoriesDb), new RealtimeService()), +); + +async function withWriteHarness(userId: number, fn: (client: Client) => Promise) { + const server = new McpServer({ name: 'trek-test', version: '1.0.0' }); + createTestRegistry([categoriesMcp], { accessPolicy: trekMcpAccessPolicy, validateAccess: trekMcpValidateAccess }) + .attach(server, { userId, scopes: null, isStaticToken: false }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + const client = new Client({ name: 'test-client', version: '1.0.0' }); + await server.connect(serverTransport); + await client.connect(clientTransport); + try { + await fn(client); + } finally { + try { await client.close(); } catch { /* ignore */ } + try { await server.close(); } catch { /* ignore */ } + } +} + +function insertCategory(name: string, color = '#111111', icon = '🅰️'): number { + return Number(testDb.prepare('INSERT INTO categories (name, color, icon) VALUES (?, ?, ?)').run(name, color, icon).lastInsertRowid); +} + // --------------------------------------------------------------------------- // list_categories // --------------------------------------------------------------------------- @@ -96,10 +158,191 @@ describe('Tool: list_categories', () => { }); // --------------------------------------------------------------------------- -// list_categories scope gating (places:read, registration-time) +// create_category (admin only, mirroring POST /api/categories) // --------------------------------------------------------------------------- -describe('Tool: list_categories — scope gating', () => { +describe('Tool: create_category', () => { + it('persists the new category and credits the admin who minted it', async () => { + const { user: admin } = createAdmin(testDb); + await withWriteHarness(admin.id, async (client) => { + const result = await client.callTool({ + name: 'create_category', + arguments: { name: 'Street food', color: '#16a34a', icon: '🍜' }, + }); + const data = parseToolResult(result) as any; + const row = testDb.prepare('SELECT name, color, icon, user_id FROM categories WHERE id = ?').get(data.category.id) as any; + expect(row).toEqual({ name: 'Street food', color: '#16a34a', icon: '🍜', user_id: admin.id }); + }); + }); + + it('falls back to the palette defaults when color and icon are omitted', async () => { + const { user: admin } = createAdmin(testDb); + await withWriteHarness(admin.id, async (client) => { + const result = await client.callTool({ name: 'create_category', arguments: { name: 'Bare minimum' } }); + const data = parseToolResult(result) as any; + const row = testDb.prepare('SELECT color, icon FROM categories WHERE id = ?').get(data.category.id) as any; + expect(row).toEqual({ color: '#6366f1', icon: '📍' }); + }); + }); + + it('refuses a non-admin and writes nothing', async () => { + const { user } = createUser(testDb); + await withWriteHarness(user.id, async (client) => { + const result = await client.callTool({ name: 'create_category', arguments: { name: 'Sneaky' } }); + expect(result.isError).toBe(true); + expect(testDb.prepare('SELECT id FROM categories WHERE name = ?').get('Sneaky')).toBeUndefined(); + }); + }); + + it('blocks demo user', async () => { + process.env.DEMO_MODE = 'true'; + const { user } = createAdmin(testDb, { email: 'demo@trek.app' }); + await withWriteHarness(user.id, async (client) => { + const result = await client.callTool({ name: 'create_category', arguments: { name: 'Demo made this' } }); + expect(result.isError).toBe(true); + expect(testDb.prepare('SELECT id FROM categories WHERE name = ?').get('Demo made this')).toBeUndefined(); + }); + }); + + it('refuses a color that is not a hex value', async () => { + const { user: admin } = createAdmin(testDb); + await withWriteHarness(admin.id, async (client) => { + const result = await client.callTool({ name: 'create_category', arguments: { name: 'Bad colour', color: 'rebeccapurple' } }); + expect(result.isError).toBe(true); + expect(testDb.prepare('SELECT id FROM categories WHERE name = ?').get('Bad colour')).toBeUndefined(); + }); + }); + + it('refuses an empty name', async () => { + const { user: admin } = createAdmin(testDb); + await withWriteHarness(admin.id, async (client) => { + const result = await client.callTool({ name: 'create_category', arguments: { name: '' } }); + expect(result.isError).toBe(true); + }); + }); +}); + +// --------------------------------------------------------------------------- +// update_category (admin only, mirroring PUT /api/categories/:id) +// --------------------------------------------------------------------------- + +describe('Tool: update_category', () => { + it('renames and recolours an existing category', async () => { + const { user: admin } = createAdmin(testDb); + const id = insertCategory('Old name', '#111111', '🅰️'); + await withWriteHarness(admin.id, async (client) => { + await client.callTool({ name: 'update_category', arguments: { categoryId: id, name: 'New name', color: '#dc2626' } }); + const row = testDb.prepare('SELECT name, color, icon FROM categories WHERE id = ?').get(id) as any; + expect(row).toEqual({ name: 'New name', color: '#dc2626', icon: '🅰️' }); + }); + }); + + it('leaves the fields it was not given alone', async () => { + const { user: admin } = createAdmin(testDb); + const id = insertCategory('Keep me', '#0891b2', '🚕'); + await withWriteHarness(admin.id, async (client) => { + await client.callTool({ name: 'update_category', arguments: { categoryId: id, icon: '🚗' } }); + const row = testDb.prepare('SELECT name, color, icon FROM categories WHERE id = ?').get(id) as any; + expect(row).toEqual({ name: 'Keep me', color: '#0891b2', icon: '🚗' }); + }); + }); + + it('reports an unknown category', async () => { + const { user: admin } = createAdmin(testDb); + await withWriteHarness(admin.id, async (client) => { + const result = await client.callTool({ name: 'update_category', arguments: { categoryId: 999999, name: 'Nope' } }); + expect(result.isError).toBe(true); + }); + }); + + it('refuses a non-admin and leaves the row untouched', async () => { + const { user } = createUser(testDb); + const id = insertCategory('Not yours', '#9333ea', '🔒'); + await withWriteHarness(user.id, async (client) => { + const result = await client.callTool({ name: 'update_category', arguments: { categoryId: id, name: 'Hijacked' } }); + expect(result.isError).toBe(true); + expect((testDb.prepare('SELECT name FROM categories WHERE id = ?').get(id) as any).name).toBe('Not yours'); + }); + }); + + it('blocks demo user', async () => { + process.env.DEMO_MODE = 'true'; + const { user } = createAdmin(testDb, { email: 'demo@trek.app' }); + const id = insertCategory('Demo untouchable', '#ea580c', '🙅'); + await withWriteHarness(user.id, async (client) => { + const result = await client.callTool({ name: 'update_category', arguments: { categoryId: id, name: 'Changed' } }); + expect(result.isError).toBe(true); + expect((testDb.prepare('SELECT name FROM categories WHERE id = ?').get(id) as any).name).toBe('Demo untouchable'); + }); + }); + + it('refuses a color that is not a hex value', async () => { + const { user: admin } = createAdmin(testDb); + const id = insertCategory('Colour guard', '#2563eb', '🎨'); + await withWriteHarness(admin.id, async (client) => { + const result = await client.callTool({ name: 'update_category', arguments: { categoryId: id, color: 'goldenrod' } }); + expect(result.isError).toBe(true); + expect((testDb.prepare('SELECT color FROM categories WHERE id = ?').get(id) as any).color).toBe('#2563eb'); + }); + }); +}); + +// --------------------------------------------------------------------------- +// delete_category (admin only, mirroring DELETE /api/categories/:id) +// --------------------------------------------------------------------------- + +describe('Tool: delete_category', () => { + it('removes the category and unassigns the places that carried it', async () => { + const { user: admin } = createAdmin(testDb); + const id = insertCategory('Doomed', '#d97706', '💀'); + const trip = Number(testDb.prepare('INSERT INTO trips (user_id, title) VALUES (?, ?)').run(admin.id, 'Trip').lastInsertRowid); + const place = Number(testDb.prepare('INSERT INTO places (trip_id, name, category_id) VALUES (?, ?, ?)').run(trip, 'Somewhere', id).lastInsertRowid); + await withWriteHarness(admin.id, async (client) => { + const result = await client.callTool({ name: 'delete_category', arguments: { categoryId: id } }); + expect((parseToolResult(result) as any).success).toBe(true); + expect(testDb.prepare('SELECT id FROM categories WHERE id = ?').get(id)).toBeUndefined(); + expect((testDb.prepare('SELECT category_id FROM places WHERE id = ?').get(place) as any).category_id).toBeNull(); + }); + }); + + it('reports an unknown category', async () => { + const { user: admin } = createAdmin(testDb); + await withWriteHarness(admin.id, async (client) => { + const result = await client.callTool({ name: 'delete_category', arguments: { categoryId: 999999 } }); + expect(result.isError).toBe(true); + }); + }); + + it('refuses a non-admin and keeps the row', async () => { + const { user } = createUser(testDb); + const id = insertCategory('Survivor', '#16a34a', '🌿'); + await withWriteHarness(user.id, async (client) => { + const result = await client.callTool({ name: 'delete_category', arguments: { categoryId: id } }); + expect(result.isError).toBe(true); + expect(testDb.prepare('SELECT id FROM categories WHERE id = ?').get(id)).toBeDefined(); + }); + }); + + it('blocks demo user', async () => { + process.env.DEMO_MODE = 'true'; + const { user } = createAdmin(testDb, { email: 'demo@trek.app' }); + const id = insertCategory('Demo survivor', '#0891b2', '🛟'); + await withWriteHarness(user.id, async (client) => { + const result = await client.callTool({ name: 'delete_category', arguments: { categoryId: id } }); + expect(result.isError).toBe(true); + expect(testDb.prepare('SELECT id FROM categories WHERE id = ?').get(id)).toBeDefined(); + }); + }); +}); + +// --------------------------------------------------------------------------- +// Scope gating (places:read for the listing, places:write for the palette +// writes, both registration-time via the declarative access markers) +// --------------------------------------------------------------------------- + +describe('Category tools: scope gating', () => { + const WRITE_TOOLS = ['create_category', 'update_category', 'delete_category']; + async function listToolNames(userId: number, scopes: string[] | null): Promise { const h = await createMcpHarness({ userId, withResources: false, scopes }); try { @@ -111,7 +354,9 @@ describe('Tool: list_categories — scope gating', () => { it('registers with null scopes (full access)', async () => { const { user } = createUser(testDb); - expect(await listToolNames(user.id, null)).toContain('list_categories'); + const names = await listToolNames(user.id, null); + expect(names).toContain('list_categories'); + for (const tool of WRITE_TOOLS) expect(names).toContain(tool); }); it('registers with places:read', async () => { @@ -119,9 +364,23 @@ describe('Tool: list_categories — scope gating', () => { expect(await listToolNames(user.id, ['places:read'])).toContain('list_categories'); }); + it('registers no palette writes with places:read only', async () => { + const { user } = createUser(testDb); + const names = await listToolNames(user.id, ['places:read']); + for (const tool of WRITE_TOOLS) expect(names).not.toContain(tool); + }); + + it('registers the palette writes with places:write', async () => { + const { user } = createUser(testDb); + const names = await listToolNames(user.id, ['places:write']); + for (const tool of WRITE_TOOLS) expect(names).toContain(tool); + }); + it('does not register for an unrelated scope', async () => { const { user } = createUser(testDb); - expect(await listToolNames(user.id, ['budget:read'])).not.toContain('list_categories'); + const names = await listToolNames(user.id, ['budget:read']); + expect(names).not.toContain('list_categories'); + for (const tool of WRITE_TOOLS) expect(names).not.toContain(tool); }); }); diff --git a/server/tests/unit/mcp/tools-notes.test.ts b/server/tests/unit/mcp/tools-notes.test.ts index b7878f42e..83f5bd5f5 100644 --- a/server/tests/unit/mcp/tools-notes.test.ts +++ b/server/tests/unit/mcp/tools-notes.test.ts @@ -115,6 +115,52 @@ describe('Tool: create_day_note', () => { }); }); + it('persists a palette color and an explicit position', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'create_day_note', + arguments: { tripId: trip.id, dayId: day.id, text: 'Ferry leaves early', color: '#dc2626', sort_order: 2 }, + }); + const data = parseToolResult(result) as any; + const row = testDb.prepare('SELECT color, sort_order FROM day_notes WHERE id = ?').get(data.note.id) as any; + expect(row.color).toBe('#dc2626'); + expect(row.sort_order).toBe(2); + }); + }); + + it('stores no color and appends at the bottom when neither is given', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'create_day_note', + arguments: { tripId: trip.id, dayId: day.id, text: 'A note' }, + }); + const data = parseToolResult(result) as any; + const row = testDb.prepare('SELECT color, sort_order FROM day_notes WHERE id = ?').get(data.note.id) as any; + expect(row.color).toBeNull(); + expect(row.sort_order).toBe(9999); + }); + }); + + it('refuses a color outside the note palette and writes nothing', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'create_day_note', + arguments: { tripId: trip.id, dayId: day.id, text: 'A note', color: '#ff0000' }, + }); + expect(result.isError).toBe(true); + expect(testDb.prepare('SELECT COUNT(*) AS n FROM day_notes WHERE day_id = ?').get(day.id)).toEqual({ n: 0 }); + }); + }); + it('returns error when day does not belong to trip', async () => { const { user } = createUser(testDb); const trip1 = createTrip(testDb, user.id); @@ -175,6 +221,69 @@ describe('Tool: update_day_note', () => { }); }); + it('sets color and moves the note within the day', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const note = createDayNote(testDb, day.id, trip.id, { sort_order: 9999 }); + await withHarness(user.id, async (h) => { + await h.client.callTool({ + name: 'update_day_note', + arguments: { tripId: trip.id, dayId: day.id, noteId: note.id, color: '#16a34a', sort_order: 1.5 }, + }); + const row = testDb.prepare('SELECT color, sort_order FROM day_notes WHERE id = ?').get(note.id) as any; + expect(row.color).toBe('#16a34a'); + expect(row.sort_order).toBe(1.5); + }); + }); + + it('clears the color with an explicit null', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const note = createDayNote(testDb, day.id, trip.id); + testDb.prepare('UPDATE day_notes SET color = ? WHERE id = ?').run('#2563eb', note.id); + await withHarness(user.id, async (h) => { + await h.client.callTool({ + name: 'update_day_note', + arguments: { tripId: trip.id, dayId: day.id, noteId: note.id, color: null }, + }); + expect((testDb.prepare('SELECT color FROM day_notes WHERE id = ?').get(note.id) as any).color).toBeNull(); + }); + }); + + it('keeps the stored color and position when neither is sent', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const note = createDayNote(testDb, day.id, trip.id, { sort_order: 3 }); + testDb.prepare('UPDATE day_notes SET color = ? WHERE id = ?').run('#9333ea', note.id); + await withHarness(user.id, async (h) => { + await h.client.callTool({ + name: 'update_day_note', + arguments: { tripId: trip.id, dayId: day.id, noteId: note.id, text: 'Only the text changed' }, + }); + const row = testDb.prepare('SELECT color, sort_order FROM day_notes WHERE id = ?').get(note.id) as any; + expect(row.color).toBe('#9333ea'); + expect(row.sort_order).toBe(3); + }); + }); + + it('refuses a color outside the note palette', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const note = createDayNote(testDb, day.id, trip.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'update_day_note', + arguments: { tripId: trip.id, dayId: day.id, noteId: note.id, color: 'red' }, + }); + expect(result.isError).toBe(true); + expect((testDb.prepare('SELECT color FROM day_notes WHERE id = ?').get(note.id) as any).color).toBeNull(); + }); + }); + it('broadcasts dayNote:updated event', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); @@ -371,6 +480,31 @@ describe('Tool: create_collab_note', () => { }); }); + it('persists the website link', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'create_collab_note', + arguments: { tripId: trip.id, title: 'Boat tour', website: 'https://example.com/tour' }, + }); + const data = parseToolResult(result) as any; + expect(data.note.website).toBe('https://example.com/tour'); + expect((testDb.prepare('SELECT website FROM collab_notes WHERE id = ?').get(data.note.id) as any).website) + .toBe('https://example.com/tour'); + }); + }); + + it('stores no website when none is given', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'create_collab_note', arguments: { tripId: trip.id, title: 'Plain note' } }); + const data = parseToolResult(result) as any; + expect((testDb.prepare('SELECT website FROM collab_notes WHERE id = ?').get(data.note.id) as any).website).toBeNull(); + }); + }); + it('broadcasts collab:note:created event', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); @@ -412,6 +546,57 @@ describe('Tool: update_collab_note', () => { }); }); + it('sets the website link on an existing note', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const note = createCollabNote(testDb, trip.id, user.id); + await withHarness(user.id, async (h) => { + await h.client.callTool({ + name: 'update_collab_note', + arguments: { tripId: trip.id, noteId: note.id, website: 'https://example.com/museum' }, + }); + expect((testDb.prepare('SELECT website FROM collab_notes WHERE id = ?').get(note.id) as any).website) + .toBe('https://example.com/museum'); + }); + }); + + it('clears the website with an explicit null', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const note = createCollabNote(testDb, trip.id, user.id); + testDb.prepare('UPDATE collab_notes SET website = ? WHERE id = ?').run('https://example.com/old', note.id); + await withHarness(user.id, async (h) => { + await h.client.callTool({ name: 'update_collab_note', arguments: { tripId: trip.id, noteId: note.id, website: null } }); + expect((testDb.prepare('SELECT website FROM collab_notes WHERE id = ?').get(note.id) as any).website).toBeNull(); + }); + }); + + it('keeps the stored website when the field is not sent', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const note = createCollabNote(testDb, trip.id, user.id); + testDb.prepare('UPDATE collab_notes SET website = ? WHERE id = ?').run('https://example.com/keep', note.id); + await withHarness(user.id, async (h) => { + await h.client.callTool({ name: 'update_collab_note', arguments: { tripId: trip.id, noteId: note.id, title: 'Renamed' } }); + expect((testDb.prepare('SELECT website FROM collab_notes WHERE id = ?').get(note.id) as any).website) + .toBe('https://example.com/keep'); + }); + }); + + it('refuses a website past the tool cap, the way the sibling text fields are capped', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const note = createCollabNote(testDb, trip.id, user.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'update_collab_note', + arguments: { tripId: trip.id, noteId: note.id, website: `https://example.com/${'x'.repeat(600)}` }, + }); + expect(result.isError).toBe(true); + expect((testDb.prepare('SELECT website FROM collab_notes WHERE id = ?').get(note.id) as any).website).toBeNull(); + }); + }); + it('broadcasts collab:note:updated event', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id);