From fd7fbef295c83d4e13645d118841bbc39f5811f5 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 22 Oct 2026 13:03:59 +0200 Subject: [PATCH] fix(ci): message part types across the strict shared build, formatting and trip member write tests --- .../components/Assistant/assistantParts.ts | 2 +- server/src/nest/assistant/cards/registry.ts | 6 +- .../trip-member-writes.service.ts | 4 +- server/tests/unit/mcp/tools-roadtrip.test.ts | 2 +- .../nest/trip-member-writes.service.test.ts | 113 ++++++++++++++++++ .../unit/reservations/bookings.rpc.test.ts | 5 +- shared/src/assistant/assistant.schema.ts | 17 ++- 7 files changed, 129 insertions(+), 20 deletions(-) create mode 100644 server/tests/unit/nest/trip-member-writes.service.test.ts diff --git a/client/src/components/Assistant/assistantParts.ts b/client/src/components/Assistant/assistantParts.ts index 76b717014..471c0be41 100644 --- a/client/src/components/Assistant/assistantParts.ts +++ b/client/src/components/Assistant/assistantParts.ts @@ -171,7 +171,7 @@ function pushCard(blocks: MessageBlock[], entry: CardEntry, group: string): void function withoutYielded(blocks: MessageBlock[], rules: CardRules): MessageBlock[] { const changes = blocks.flatMap((b) => (b.kind === 'change' ? b.items.flatMap((i) => (i.card ? [i.card] : [])) : [])); if (changes.length === 0) return blocks; - return blocks.flatMap((block) => { + return blocks.flatMap((block): MessageBlock[] => { if (block.kind !== 'cards') return [block]; const entries = block.entries.filter((e) => !changes.some((change) => rules.yieldsToChange(e.card, change))); return entries.length > 0 ? [{ ...block, entries }] : []; diff --git a/server/src/nest/assistant/cards/registry.ts b/server/src/nest/assistant/cards/registry.ts index 5adba726b..ac4187e1f 100644 --- a/server/src/nest/assistant/cards/registry.ts +++ b/server/src/nest/assistant/cards/registry.ts @@ -330,11 +330,7 @@ export function nameArgOf(tool: string, key: string, value: number, card: TripCa } /** The reads an undo of `tool` makes before its check; none when its package asks for none or fails to say. */ -export function undoReadsOf( - tool: string, - args: ToolArgs, - result: unknown, -): UndoReads { +export function undoReadsOf(tool: string, args: ToolArgs, result: unknown): UndoReads { return TOOL_CARDS[tool]?.undo?.reads?.(args, result) ?? []; } diff --git a/server/src/nest/trip-members/trip-member-writes.service.ts b/server/src/nest/trip-members/trip-member-writes.service.ts index 58a735728..6222f36ce 100644 --- a/server/src/nest/trip-members/trip-member-writes.service.ts +++ b/server/src/nest/trip-members/trip-member-writes.service.ts @@ -73,9 +73,7 @@ export class TripMemberWritesService { async deleteGuest(tripId: string | number, guestId: number, writer: TripWriter): Promise { // Read first: the rows that name the guest go with the guest. const shared = (await this.budget.listBudgetItems(tripId)) - .filter( - (item) => item.paid_by_user_id === guestId || item.members?.some((member) => member.user_id === guestId), - ) + .filter((item) => item.paid_by_user_id === guestId || item.members?.some((member) => member.user_id === guestId)) .map((item) => item.id); if (!(await this.roster.deleteGuest(tripId, guestId))) return false; writer.events.emit('member:removed', { userId: guestId }); diff --git a/server/tests/unit/mcp/tools-roadtrip.test.ts b/server/tests/unit/mcp/tools-roadtrip.test.ts index 6790a4fd1..b220078a2 100644 --- a/server/tests/unit/mcp/tools-roadtrip.test.ts +++ b/server/tests/unit/mcp/tools-roadtrip.test.ts @@ -15,10 +15,10 @@ import { RoadtripPreferences } from '../../../src/db/entities/RoadtripPreference import { RoadtripVias } from '../../../src/db/entities/RoadtripVias.entity'; import { DEMO_EMAIL_PRIMARY } from '../../../src/nest/common/demo'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; -import { FakeRealtimeService } from '../../helpers/fake-realtime'; import { createUser, createTrip, createDay, createPlace } from '../../helpers/factories'; import { addTripMember } from '../../helpers/factories'; import { countRows, findRow, updateRows } from '../../helpers/factories/rows'; +import { FakeRealtimeService } from '../../helpers/fake-realtime'; import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness'; import { resetTestDb, setAddonEnabled } from '../../helpers/test-db'; import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; diff --git a/server/tests/unit/nest/trip-member-writes.service.test.ts b/server/tests/unit/nest/trip-member-writes.service.test.ts new file mode 100644 index 000000000..a95775947 --- /dev/null +++ b/server/tests/unit/nest/trip-member-writes.service.test.ts @@ -0,0 +1,113 @@ +/** + * TripMemberWritesService over a stubbed roster and budget. MEMBER-WRITES-001+. + * + * The routes and the MCP tools reach these use cases with rows that exist; this + * file pins the two fallbacks they never meet: an inviter whose address cannot + * be read still sends the invitation, and an expense that is gone by the time a + * deleted guest's split is read back again is left out instead of announced. + */ +import { BudgetService } from '../../../src/nest/budget/budget.service'; +import type { TripWriter } from '../../../src/nest/common/trip-writer'; +import { TripMemberWritesService } from '../../../src/nest/trip-members/trip-member-writes.service'; +import { TripMembersModule } from '../../../src/nest/trip-members/trip-members.module'; +import { TripMembersService } from '../../../src/nest/trip-members/trip-members.service'; +import { createSnapshotTestDb } from '../../helpers/db-mock'; +import { createTestModule, type TestModule } from '../../helpers/test-module'; + +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; + +const db = createSnapshotTestDb(); +let t: TestModule; +let writes: TripMemberWritesService; + +const roster = { + addMember: vi.fn(), + notifyInvite: vi.fn(), + deleteGuest: vi.fn(), +}; +const budget = { + listBudgetItems: vi.fn(), + getBudgetItem: vi.fn(), +}; + +/** A writer whose user has no row, so nothing about them can be read back. */ +function writerFor(userId: number): TripWriter { + return { userId, role: 'user', surface: 'rest', events: { emit: vi.fn(), emitAll: vi.fn() } }; +} + +beforeAll(async () => { + t = await createTestModule({ + db, + imports: [TripMembersModule], + overrides: [ + { provide: TripMembersService, useValue: roster }, + { provide: BudgetService, useValue: budget }, + ], + }); + writes = t.get(TripMemberWritesService); +}); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +afterAll(async () => { + await t.close(); + db.close(); +}); + +describe('TripMemberWritesService', () => { + it('MEMBER-WRITES-001: an inviter whose address cannot be read still sends the invitation, with an empty address', async () => { + const member = { id: 21, email: 'invitee@example.test' }; + roster.addMember.mockResolvedValue({ member, targetUserId: 21, tripTitle: 'Lisbon' }); + const writer = writerFor(987654); + + const result = await writes.addMember(7, 'invitee@example.test', 3, writer); + + expect(result.member).toBe(member); + expect(roster.addMember).toHaveBeenCalledWith(7, 'invitee@example.test', 3, 987654); + expect(writer.events.emit).toHaveBeenCalledWith('member:added', { member }); + expect(roster.notifyInvite).toHaveBeenCalledWith( + '7', + { id: 987654, email: '' }, + 21, + 'Lisbon', + 'invitee@example.test', + ); + }); + + it('MEMBER-WRITES-002: deleting a guest announces only the expenses that still read back after the re-split', async () => { + const guestId = 40; + budget.listBudgetItems.mockResolvedValue([ + { id: 1, paid_by_user_id: guestId, members: [] }, + { id: 2, paid_by_user_id: null, members: [{ user_id: guestId }] }, + { id: 3, paid_by_user_id: null, members: [{ user_id: 5 }] }, + { id: 4, paid_by_user_id: null }, + ]); + roster.deleteGuest.mockResolvedValue(true); + budget.getBudgetItem.mockImplementation((itemId: number) => (itemId === 1 ? { id: 1 } : null)); + const writer = writerFor(3); + + expect(await writes.deleteGuest(7, guestId, writer)).toBe(true); + + expect(budget.getBudgetItem.mock.calls).toEqual([ + [1, 7], + [2, 7], + ]); + expect(vi.mocked(writer.events.emit).mock.calls).toEqual([ + ['member:removed', { userId: guestId }], + ['budget:updated', { item: { id: 1 } }], + ]); + }); + + it('MEMBER-WRITES-003: a guest the roster does not delete reads nothing back and announces nothing', async () => { + budget.listBudgetItems.mockResolvedValue([{ id: 1, paid_by_user_id: 40, members: [] }]); + roster.deleteGuest.mockResolvedValue(false); + const writer = writerFor(3); + + expect(await writes.deleteGuest(7, 40, writer)).toBe(false); + + expect(budget.getBudgetItem).not.toHaveBeenCalled(); + expect(writer.events.emit).not.toHaveBeenCalled(); + }); +}); diff --git a/server/tests/unit/reservations/bookings.rpc.test.ts b/server/tests/unit/reservations/bookings.rpc.test.ts index 0d8ce434d..08f178e80 100644 --- a/server/tests/unit/reservations/bookings.rpc.test.ts +++ b/server/tests/unit/reservations/bookings.rpc.test.ts @@ -290,10 +290,7 @@ describe('ReservationsRpc', () => { const f = build({ created: true }); await f .host() - .dispatch( - req('reservations.update', { tripId: 1, reservationId: 5, input: { title: 'x', type: 'hotel' } }), - 42, - ); + .dispatch(req('reservations.update', { tripId: 1, reservationId: 5, input: { title: 'x', type: 'hotel' } }), 42); expect(events(f.realtime)).toEqual(['accommodation:created', 'reservation:updated']); }); diff --git a/shared/src/assistant/assistant.schema.ts b/shared/src/assistant/assistant.schema.ts index 8a98bdd1d..a956cd131 100644 --- a/shared/src/assistant/assistant.schema.ts +++ b/shared/src/assistant/assistant.schema.ts @@ -542,12 +542,17 @@ export const assistantToolApprovalSchema = z.object({ }); export type AssistantToolApproval = z.infer; -/** The parts of a message, with the change, receipt and card parts read or written as given. */ -function messagePartSchema( - change: z.ZodType, - applied: z.ZodType, - card: z.ZodType, -) { +/** + * The parts of a message, with the change, receipt and card parts read or + * written as given. Generic over the three schemas so the emitted declaration + * keeps each one's own Zod type instead of a precomputed object type, which a + * consumer without strictNullChecks would read differently from its own infer. + */ +function messagePartSchema< + Change extends z.ZodType, + Applied extends z.ZodType, + Card extends z.ZodType, +>(change: Change, applied: Applied, card: Card) { return z.discriminatedUnion('type', [ z.object({ type: z.literal('text'), text: z.string() }), // A file the user handed the assistant. Only its description is kept, never its content.