From 6a6e98b5f76cf41f528572d01027d487aeacb099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Mon, 30 Mar 2026 12:03:27 +0200 Subject: [PATCH] fix(api): harden broadcast and assistant fallback errors --- .../assistant-tools-advanced.test.ts | 45 ++++++ .../assistant-tools-import-export.test.ts | 96 ++++++++++++- .../src/__tests__/notification-router.test.ts | 8 +- packages/api/src/router/assistant-tools.ts | 132 ++++++++++++++---- packages/api/src/router/notification.ts | 81 ++++++++--- 5 files changed, 305 insertions(+), 57 deletions(-) diff --git a/packages/api/src/__tests__/assistant-tools-advanced.test.ts b/packages/api/src/__tests__/assistant-tools-advanced.test.ts index bafde9e..9f7151c 100644 --- a/packages/api/src/__tests__/assistant-tools-advanced.test.ts +++ b/packages/api/src/__tests__/assistant-tools-advanced.test.ts @@ -1374,6 +1374,51 @@ describe("assistant advanced tools and scoping", () => { }); }); + it("returns a stable demand-requirement-not-found error for batch timeline shifts", async () => { + const demandRequirement = { + id: "demand_requirement_1", + startDate: new Date("2026-03-10"), + endDate: new Date("2026-03-14"), + }; + const db = { + allocation: { + findUnique: vi.fn().mockResolvedValue(null), + }, + demandRequirement: { + findUnique: vi.fn().mockResolvedValue(demandRequirement), + }, + assignment: { + findUnique: vi.fn().mockResolvedValue(null), + }, + $transaction: vi.fn(async () => { + throw new TRPCError({ + code: "NOT_FOUND", + message: "Demand requirement not found", + }); + }), + }; + const ctx = createToolContext( + db, + [PermissionKey.MANAGE_ALLOCATIONS, PermissionKey.USE_ASSISTANT_ADVANCED_TOOLS], + SystemRole.MANAGER, + ); + + const result = await executeTool( + "batch_shift_timeline_allocations", + JSON.stringify({ + allocationIds: ["demand_requirement_missing"], + daysDelta: 3, + mode: "move", + }), + ctx, + ); + + expect(result.action).toBeUndefined(); + expect(JSON.parse(result.content)).toEqual({ + error: "Demand requirement not found with the given criteria.", + }); + }); + it("returns the chargeability report readmodel through the assistant", async () => { const { listAssignmentBookings } = await import("@capakraken/application"); vi.mocked(listAssignmentBookings).mockResolvedValue([ diff --git a/packages/api/src/__tests__/assistant-tools-import-export.test.ts b/packages/api/src/__tests__/assistant-tools-import-export.test.ts index 698d74d..f152ba9 100644 --- a/packages/api/src/__tests__/assistant-tools-import-export.test.ts +++ b/packages/api/src/__tests__/assistant-tools-import-export.test.ts @@ -2170,14 +2170,15 @@ describe("assistant import/export and dispo tools", () => { }); it("returns a stable assistant error when a broadcast target resolves to no recipients", async () => { + const create = vi.fn().mockResolvedValue({ + id: "broadcast_empty", + title: "Office update", + targetType: "user", + }); const ctx = createToolContext( { notificationBroadcast: { - create: vi.fn().mockResolvedValue({ - id: "broadcast_empty", - title: "Office update", - targetType: "user", - }), + create, }, }, { userRole: SystemRole.MANAGER }, @@ -2195,6 +2196,39 @@ describe("assistant import/export and dispo tools", () => { expect(JSON.parse(result.content)).toEqual({ error: "No recipients matched the broadcast target.", }); + expect(create).not.toHaveBeenCalled(); + }); + + it("returns a stable assistant error when broadcast creation fails because the sender user is missing", async () => { + const ctx = createToolContext( + { + user: { + findMany: vi.fn().mockResolvedValue([{ id: "user_2" }]), + }, + notificationBroadcast: { + create: vi.fn().mockRejectedValue( + Object.assign(new Error("Foreign key constraint failed"), { + code: "P2003", + meta: { field_name: "NotificationBroadcast_senderId_fkey" }, + }), + ), + }, + }, + { userRole: SystemRole.MANAGER }, + ); + + const result = await executeTool( + "send_broadcast", + JSON.stringify({ + title: "Office update", + targetType: "all", + }), + ctx, + ); + + expect(JSON.parse(result.content)).toEqual({ + error: "Sender user not found with the given criteria.", + }); }); it("reads broadcast details through the real notification router and rejects plain users", async () => { @@ -2770,6 +2804,58 @@ describe("assistant import/export and dispo tools", () => { setup: () => vi.mocked(updateEstimateDraft).mockRejectedValueOnce(new Error("Estimate has no working version")), expected: "Estimate has no working version.", }, + { + name: "update_estimate_draft missing scope item reference", + toolName: "update_estimate_draft", + payload: { + id: "est_scope_missing", + baseCurrency: "EUR", + assumptions: [], + scopeItems: [], + demandLines: [], + resourceSnapshots: [], + metrics: [], + }, + permission: PermissionKey.MANAGE_PROJECTS, + db: { + estimate: { + findUnique: vi.fn().mockResolvedValue({ projectId: null }), + }, + }, + setup: () => vi.mocked(updateEstimateDraft).mockRejectedValueOnce( + Object.assign(new Error("Foreign key constraint failed"), { + code: "P2003", + meta: { field_name: "EstimateScopeItem_scopeItemId_fkey" }, + }), + ), + expected: "Estimate scope item not found with the given criteria.", + }, + { + name: "update_estimate_draft generic missing estimate reference", + toolName: "update_estimate_draft", + payload: { + id: "est_reference_missing", + baseCurrency: "EUR", + assumptions: [], + scopeItems: [], + demandLines: [], + resourceSnapshots: [], + metrics: [], + }, + permission: PermissionKey.MANAGE_PROJECTS, + db: { + estimate: { + findUnique: vi.fn().mockResolvedValue({ projectId: null }), + }, + }, + setup: () => vi.mocked(updateEstimateDraft).mockRejectedValueOnce( + Object.assign(new Error("Foreign key constraint failed"), { + code: "P2003", + meta: { field_name: "EstimateVersion_estimateId_fkey" }, + }), + ), + expected: "One of the referenced project, role, resource, or scope items no longer exists.", + }, { name: "submit_estimate_version missing version", toolName: "submit_estimate_version", diff --git a/packages/api/src/__tests__/notification-router.test.ts b/packages/api/src/__tests__/notification-router.test.ts index f0f6ae1..d1e7220 100644 --- a/packages/api/src/__tests__/notification-router.test.ts +++ b/packages/api/src/__tests__/notification-router.test.ts @@ -306,13 +306,7 @@ describe("notification.createBroadcast", () => { message: "No recipients matched the broadcast target.", }); - expect(create).toHaveBeenCalledWith(expect.objectContaining({ - data: expect.objectContaining({ - senderId: "user_mgr", - title: "Ops update", - targetType: "all", - }), - })); + expect(create).not.toHaveBeenCalled(); expect(update).not.toHaveBeenCalled(); expect(resolveRecipientsMock).toHaveBeenCalledWith( "all", diff --git a/packages/api/src/router/assistant-tools.ts b/packages/api/src/router/assistant-tools.ts index 067d687..d19c306 100644 --- a/packages/api/src/router/assistant-tools.ts +++ b/packages/api/src/router/assistant-tools.ts @@ -463,11 +463,6 @@ function toAssistantTimelineMutationError( error: unknown, context: "updateInline" | "applyShift" | "quickAssign" | "batchShift", ): AssistantToolErrorResult | null { - const allocationNotFound = toAssistantAllocationNotFoundError(error); - if (allocationNotFound && (context === "updateInline" || context === "batchShift")) { - return allocationNotFound; - } - if (error instanceof TRPCError) { if (error.code === "NOT_FOUND") { if (error.message.includes("Resource not found")) { @@ -489,6 +484,11 @@ function toAssistantTimelineMutationError( } } + const allocationNotFound = toAssistantAllocationNotFoundError(error); + if (allocationNotFound && (context === "updateInline" || context === "batchShift")) { + return allocationNotFound; + } + const prismaError = getPrismaRequestErrorMetadata(error); if (!prismaError) { return null; @@ -1115,21 +1115,11 @@ function toAssistantEstimateMutationError( return null; } - const errorText = `${prismaError.message} ${prismaError.metaText}`.toLowerCase(); - if (errorText.includes("project")) { - return { error: "Project not found with the given criteria." }; - } - if (errorText.includes("estimatedemandline") || errorText.includes("estimate_demand_line") || errorText.includes("estimate demand line")) { - return { error: "Estimate demand line not found with the given criteria." }; - } - if (errorText.includes("estimateversion") || errorText.includes("estimate_version") || errorText.includes("estimate version")) { - return { error: "Estimate version not found with the given criteria." }; - } - if (errorText.includes("estimate")) { - return { error: "Estimate not found with the given criteria." }; - } - if (prismaError.code === "P2003") { + const errorText = `${prismaError.message} ${prismaError.metaText}`.toLowerCase(); + if (errorText.includes("project")) { + return { error: "Project not found with the given criteria." }; + } if (errorText.includes("role")) { return { error: "Role not found with the given criteria." }; } @@ -1139,9 +1129,20 @@ function toAssistantEstimateMutationError( if (errorText.includes("scopeitem") || errorText.includes("scope_item") || errorText.includes("scope item")) { return { error: "Estimate scope item not found with the given criteria." }; } + return { error: "One of the referenced project, role, resource, or scope items no longer exists." }; } if (prismaError.code === "P2025") { + const errorText = `${prismaError.message} ${prismaError.metaText}`.toLowerCase(); + if (errorText.includes("estimatedemandline") || errorText.includes("estimate_demand_line") || errorText.includes("estimate demand line")) { + return { error: "Estimate demand line not found with the given criteria." }; + } + if (errorText.includes("estimateversion") || errorText.includes("estimate_version") || errorText.includes("estimate version")) { + return { error: "Estimate version not found with the given criteria." }; + } + if (errorText.includes("estimate")) { + return { error: "Estimate not found with the given criteria." }; + } switch (action) { case "generateWeeklyPhasing": return { error: "Estimate demand line not found with the given criteria." }; @@ -1631,19 +1632,97 @@ function getPrismaRequestErrorMetadata(error: unknown): { return null; } +function getTrpcErrorMetadata(error: unknown): { + code: string; + message: string; +} | null { + const queue: unknown[] = [error]; + const visited = new Set(); + + while (queue.length > 0) { + const current = queue.shift(); + if (current === undefined || current === null || visited.has(current)) { + continue; + } + visited.add(current); + + if (current instanceof TRPCError) { + return { + code: current.code, + message: current.message, + }; + } + + if (typeof current !== "object") { + continue; + } + + const candidate = current as { + code?: unknown; + message?: unknown; + cause?: unknown; + data?: { code?: unknown }; + shape?: { code?: unknown; message?: unknown }; + }; + + const candidateCode = typeof candidate.code === "string" + ? candidate.code + : typeof candidate.data?.code === "string" + ? candidate.data.code + : typeof candidate.shape?.code === "string" + ? candidate.shape.code + : null; + const candidateMessage = typeof candidate.message === "string" + ? candidate.message + : typeof candidate.shape?.message === "string" + ? candidate.shape.message + : ""; + + if (candidateCode && /^[A-Z_]+$/.test(candidateCode)) { + return { + code: candidateCode, + message: candidateMessage, + }; + } + + if ("cause" in candidate) { + queue.push(candidate.cause); + } + } + + return null; +} + function toAssistantNotificationCreationError( error: unknown, context: "notification" | "task" | "broadcast", ): AssistantToolErrorResult | null { + const trpcError = getTrpcErrorMetadata(error); + if ( context === "broadcast" - && error instanceof TRPCError - && error.code === "BAD_REQUEST" - && error.message === "No recipients matched the broadcast target." + && trpcError?.code === "BAD_REQUEST" + && trpcError.message === "No recipients matched the broadcast target." ) { return { error: "No recipients matched the broadcast target." }; } + if (trpcError?.code === "NOT_FOUND") { + if (trpcError.message.includes("Sender user not found")) { + return { error: "Sender user not found with the given criteria." }; + } + if (trpcError.message.includes("Assignee user not found")) { + return { error: "Assignee user not found with the given criteria." }; + } + if (trpcError.message.includes("recipient")) { + return context === "broadcast" + ? { error: "Broadcast recipient user not found with the given criteria." } + : context === "task" + ? { error: "Task recipient user not found with the given criteria." } + : { error: "Notification recipient user not found with the given criteria." }; + } + } + const prismaError = getPrismaRequestErrorMetadata(error); if (!prismaError) { return null; @@ -1681,20 +1760,21 @@ function normalizeAssistantExecutionError( return { error: error.message }; } - if (error instanceof TRPCError) { - if (error.code === "INTERNAL_SERVER_ERROR") { + const trpcError = getTrpcErrorMetadata(error); + if (trpcError) { + if (trpcError.code === "INTERNAL_SERVER_ERROR") { return { error: "The tool could not complete due to an internal error.", }; } - if (error.code === "UNAUTHORIZED") { + if (trpcError.code === "UNAUTHORIZED") { return { error: "Authentication is required to use this tool.", }; } - if (error.code === "FORBIDDEN") { + if (trpcError.code === "FORBIDDEN") { return { error: "You do not have permission to perform this action.", }; diff --git a/packages/api/src/router/notification.ts b/packages/api/src/router/notification.ts index 09ab7db..b4cd957 100644 --- a/packages/api/src/router/notification.ts +++ b/packages/api/src/router/notification.ts @@ -63,6 +63,31 @@ async function sendNotificationEmail( } } +function rethrowNotificationReferenceError(error: unknown): never { + const candidate = error as { + code?: unknown; + message?: unknown; + meta?: { field_name?: unknown }; + }; + const fieldName = typeof candidate.meta?.field_name === "string" + ? candidate.meta.field_name.toLowerCase() + : ""; + + if ( + typeof candidate.code === "string" + && (candidate.code === "P2003" || candidate.code === "P2025") + && fieldName.includes("sender") + ) { + throw new TRPCError({ + code: "NOT_FOUND", + message: "Sender user not found", + cause: error, + }); + } + + throw error; +} + // ─── Zod Enums ──────────────────────────────────────────────────────────────── const categoryEnum = z.enum(["NOTIFICATION", "REMINDER", "TASK", "APPROVAL"]); @@ -590,28 +615,26 @@ export const notificationRouter = createTRPCRouter({ .mutation(async ({ ctx, input }) => { const senderId = ctx.dbUser.id; - // 1. Create broadcast record - const broadcast = await ctx.db.notificationBroadcast.create({ - data: { - senderId, - title: input.title, - ...(input.body !== undefined ? { body: input.body } : {}), - ...(input.link !== undefined ? { link: input.link } : {}), - category: input.category, - priority: input.priority, - channel: input.channel, - targetType: input.targetType, - ...(input.targetValue !== undefined ? { targetValue: input.targetValue } : {}), - ...(input.scheduledAt !== undefined ? { scheduledAt: input.scheduledAt } : {}), - }, - }); - - // 2. If scheduled in the future, just return the broadcast + // Scheduled broadcasts can be stored immediately because fan-out is deferred. if (input.scheduledAt && input.scheduledAt > new Date()) { - return broadcast; + return ctx.db.notificationBroadcast.create({ + data: { + senderId, + title: input.title, + ...(input.body !== undefined ? { body: input.body } : {}), + ...(input.link !== undefined ? { link: input.link } : {}), + category: input.category, + priority: input.priority, + channel: input.channel, + targetType: input.targetType, + ...(input.targetValue !== undefined ? { targetValue: input.targetValue } : {}), + ...(input.scheduledAt !== undefined ? { scheduledAt: input.scheduledAt } : {}), + }, + }); } - // 3. Resolve recipients + // Resolve recipients before persisting immediate broadcasts so empty targets + // do not leave orphaned broadcast rows behind. const recipientIds = await resolveRecipients( input.targetType, input.targetValue, @@ -626,6 +649,26 @@ export const notificationRouter = createTRPCRouter({ }); } + let broadcast; + try { + broadcast = await ctx.db.notificationBroadcast.create({ + data: { + senderId, + title: input.title, + ...(input.body !== undefined ? { body: input.body } : {}), + ...(input.link !== undefined ? { link: input.link } : {}), + category: input.category, + priority: input.priority, + channel: input.channel, + targetType: input.targetType, + ...(input.targetValue !== undefined ? { targetValue: input.targetValue } : {}), + ...(input.scheduledAt !== undefined ? { scheduledAt: input.scheduledAt } : {}), + }, + }); + } catch (error) { + rethrowNotificationReferenceError(error); + } + // 4. Create individual notifications for each recipient const isTask = input.category === "TASK" || input.category === "APPROVAL";