From 3743cf7fc8507aadbebcc0dc154d4ed003d984ad Mon Sep 17 00:00:00 2001 From: Kamil Galuszka Date: Wed, 1 Apr 2026 21:47:33 +0200 Subject: [PATCH 1/3] refactor(calendar): moving validation logic outside of service - implements #313 --- skills/google-calendar/SKILL.md | 4 +- .../services/CalendarService.test.ts | 83 +++++-- .../services/CalendarValidation.test.ts | 151 +++++++++++++ workspace-server/src/index.ts | 75 +++---- .../src/services/CalendarService.ts | 181 ++------------- .../src/services/CalendarValidation.ts | 207 ++++++++++++++++++ 6 files changed, 479 insertions(+), 222 deletions(-) create mode 100644 workspace-server/src/__tests__/services/CalendarValidation.test.ts create mode 100644 workspace-server/src/services/CalendarValidation.ts diff --git a/skills/google-calendar/SKILL.md b/skills/google-calendar/SKILL.md index b774220..2a32ed9 100644 --- a/skills/google-calendar/SKILL.md +++ b/skills/google-calendar/SKILL.md @@ -234,6 +234,8 @@ calendar.createEvent({ ``` - **`summary`** defaults to `"Working Location"` if omitted +- **All-day working location events** must span **exactly one day**. Use the + next day as the exclusive `end` date. - **`workingLocationProperties`** is **required** when `eventType` is `"workingLocation"` - **`workingLocationProperties.type`** — `"homeOffice"`, `"officeLocation"`, or @@ -268,7 +270,7 @@ be changed — everything else is preserved. - **Changing title/description**: Update `summary` or `description` - **Adding Google Meet**: Set `addGoogleMeet: true` to generate a Meet link - **Managing attachments**: Provide the full attachment list (replaces all - existing) + existing). Pass `attachments: []` to clear all attachments. > **Important:** The `attendees` field is a full replacement, not an append. To > add a new attendee, include all existing attendees plus the new one. The same diff --git a/workspace-server/src/__tests__/services/CalendarService.test.ts b/workspace-server/src/__tests__/services/CalendarService.test.ts index 94fde5e..d405cf4 100644 --- a/workspace-server/src/__tests__/services/CalendarService.test.ts +++ b/workspace-server/src/__tests__/services/CalendarService.test.ts @@ -857,7 +857,7 @@ describe('CalendarService', () => { attendees: [{ email: 'new@example.com' }], }; - mockCalendarAPI.events.update.mockResolvedValue({ data: updatedEvent }); + mockCalendarAPI.events.patch.mockResolvedValue({ data: updatedEvent }); const result = await calendarService.updateEvent({ eventId: 'event123', @@ -867,7 +867,7 @@ describe('CalendarService', () => { attendees: ['new@example.com'], }); - expect(mockCalendarAPI.events.update).toHaveBeenCalledWith({ + expect(mockCalendarAPI.events.patch).toHaveBeenCalledWith({ calendarId: 'primary', eventId: 'event123', requestBody: { @@ -889,14 +889,14 @@ describe('CalendarService', () => { description: 'New updated description', }; - mockCalendarAPI.events.update.mockResolvedValue({ data: updatedEvent }); + mockCalendarAPI.events.patch.mockResolvedValue({ data: updatedEvent }); const result = await calendarService.updateEvent({ eventId: 'event123', description: 'New updated description', }); - expect(mockCalendarAPI.events.update).toHaveBeenCalledWith({ + expect(mockCalendarAPI.events.patch).toHaveBeenCalledWith({ calendarId: 'primary', eventId: 'event123', requestBody: { @@ -910,7 +910,7 @@ describe('CalendarService', () => { it('should handle update errors', async () => { const apiError = new Error('Update failed'); - mockCalendarAPI.events.update.mockRejectedValue(apiError); + mockCalendarAPI.events.patch.mockRejectedValue(apiError); const result = await calendarService.updateEvent({ eventId: 'event123', @@ -927,14 +927,14 @@ describe('CalendarService', () => { summary: 'Updated Meeting Only', }; - mockCalendarAPI.events.update.mockResolvedValue({ data: updatedEvent }); + mockCalendarAPI.events.patch.mockResolvedValue({ data: updatedEvent }); await calendarService.updateEvent({ eventId: 'event123', summary: 'Updated Meeting Only', }); - expect(mockCalendarAPI.events.update).toHaveBeenCalledWith({ + expect(mockCalendarAPI.events.patch).toHaveBeenCalledWith({ calendarId: 'primary', eventId: 'event123', requestBody: { @@ -942,6 +942,28 @@ describe('CalendarService', () => { }, }); }); + + it('should patch only changed fields for status events', async () => { + mockCalendarAPI.events.patch.mockResolvedValue({ + data: { + id: 'focus123', + summary: 'Deep Work', + }, + }); + + await calendarService.updateEvent({ + eventId: 'focus123', + summary: 'Deep Work', + }); + + expect(mockCalendarAPI.events.patch).toHaveBeenCalledWith({ + calendarId: 'primary', + eventId: 'focus123', + requestBody: { + summary: 'Deep Work', + }, + }); + }); }); describe('respondToEvent', () => { @@ -1495,14 +1517,14 @@ describe('CalendarService', () => { }, }; - mockCalendarAPI.events.update.mockResolvedValue({ data: updatedEvent }); + mockCalendarAPI.events.patch.mockResolvedValue({ data: updatedEvent }); const result = await calendarService.updateEvent({ eventId: 'event123', addGoogleMeet: true, }); - const callArgs = mockCalendarAPI.events.update.mock.calls[0][0]; + const callArgs = mockCalendarAPI.events.patch.mock.calls[0][0]; expect(callArgs.conferenceDataVersion).toBe(1); expect(callArgs.requestBody.conferenceData).toBeDefined(); expect( @@ -1515,7 +1537,7 @@ describe('CalendarService', () => { it('should not include conferenceData when addGoogleMeet is false', async () => { const updatedEvent = { id: 'event123', summary: 'No Meet' }; - mockCalendarAPI.events.update.mockResolvedValue({ data: updatedEvent }); + mockCalendarAPI.events.patch.mockResolvedValue({ data: updatedEvent }); await calendarService.updateEvent({ eventId: 'event123', @@ -1523,7 +1545,7 @@ describe('CalendarService', () => { addGoogleMeet: false, }); - const callArgs = mockCalendarAPI.events.update.mock.calls[0][0]; + const callArgs = mockCalendarAPI.events.patch.mock.calls[0][0]; expect(callArgs.conferenceDataVersion).toBeUndefined(); expect(callArgs.requestBody.conferenceData).toBeUndefined(); }); @@ -1541,7 +1563,7 @@ describe('CalendarService', () => { ], }; - mockCalendarAPI.events.update.mockResolvedValue({ data: updatedEvent }); + mockCalendarAPI.events.patch.mockResolvedValue({ data: updatedEvent }); const result = await calendarService.updateEvent({ eventId: 'event123', @@ -1553,7 +1575,7 @@ describe('CalendarService', () => { ], }); - const callArgs = mockCalendarAPI.events.update.mock.calls[0][0]; + const callArgs = mockCalendarAPI.events.patch.mock.calls[0][0]; expect(callArgs.supportsAttachments).toBe(true); expect(callArgs.requestBody.attachments).toEqual([ expect.objectContaining({ @@ -1564,6 +1586,26 @@ describe('CalendarService', () => { expect(JSON.parse(result.content[0].text)).toEqual(updatedEvent); }); + + it('should clear attachments when passed an empty array', async () => { + mockCalendarAPI.events.patch.mockResolvedValue({ + data: { id: 'event123', attachments: [] }, + }); + + await calendarService.updateEvent({ + eventId: 'event123', + attachments: [], + }); + + expect(mockCalendarAPI.events.patch).toHaveBeenCalledWith({ + calendarId: 'primary', + eventId: 'event123', + supportsAttachments: true, + requestBody: { + attachments: [], + }, + }); + }); }); }); @@ -2121,6 +2163,21 @@ describe('CalendarService', () => { expect(parsedResult.error).toBe('Invalid input format'); }); + it('should reject all-day workingLocation events that span multiple days', async () => { + const result = await calendarService.createEvent({ + start: { date: '2024-01-15' }, + end: { date: '2024-01-17' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'homeOffice' }, + }); + + const parsedResult = JSON.parse(result.content[0].text); + expect(parsedResult.error).toBe('Invalid input format'); + expect(parsedResult.details).toContain( + 'all-day workingLocation events must span exactly one day', + ); + }); + it('should reject start with both dateTime and date', async () => { const result = await calendarService.createEvent({ summary: 'Ambiguous Event', diff --git a/workspace-server/src/__tests__/services/CalendarValidation.test.ts b/workspace-server/src/__tests__/services/CalendarValidation.test.ts new file mode 100644 index 0000000..7805f38 --- /dev/null +++ b/workspace-server/src/__tests__/services/CalendarValidation.test.ts @@ -0,0 +1,151 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from '@jest/globals'; +import { z } from 'zod'; +import { + validateCreateEventInput, + validateUpdateEventInput, +} from '../../services/CalendarValidation'; + +function getZodIssueMessages(fn: () => void): string[] { + try { + fn(); + return []; + } catch (error) { + if (error instanceof z.ZodError) { + return error.issues.map((issue) => issue.message); + } + throw error; + } +} + +describe('CalendarValidation', () => { + describe('validateCreateEventInput', () => { + it('accepts a single-day all-day working location event', () => { + expect(() => + validateCreateEventInput({ + start: { date: '2024-01-15' }, + end: { date: '2024-01-16' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'homeOffice' }, + }), + ).not.toThrow(); + }); + + it('rejects an all-day working location event that spans multiple days', () => { + expect(() => + validateCreateEventInput({ + start: { date: '2024-01-15' }, + end: { date: '2024-01-17' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'homeOffice' }, + }), + ).toThrow( + 'all-day workingLocation events must span exactly one day', + ); + }); + + it('accepts a leap-day working location event that spans one day', () => { + expect(() => + validateCreateEventInput({ + start: { date: '2024-02-29' }, + end: { date: '2024-03-01' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'homeOffice' }, + }), + ).not.toThrow(); + }); + + it('rejects a regular event without a summary', () => { + expect(() => + validateCreateEventInput({ + start: { dateTime: '2024-01-15T10:00:00Z' }, + end: { dateTime: '2024-01-15T11:00:00Z' }, + }), + ).toThrow('summary is required for regular events'); + }); + + it('rejects focus time events with all-day dates', () => { + expect(() => + validateCreateEventInput({ + start: { date: '2024-01-15' }, + end: { date: '2024-01-16' }, + eventType: 'focusTime', + }), + ).toThrow('focusTime events cannot be all-day events'); + }); + + it('rejects working location officeLocation without office details', () => { + const messages = getZodIssueMessages(() => + validateCreateEventInput({ + start: { date: '2024-01-15' }, + end: { date: '2024-01-16' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'officeLocation' }, + }), + ); + + expect(messages).toContain( + 'officeLocation is required when workingLocationProperties.type is "officeLocation"', + ); + }); + + it('rejects invalid attendee emails', () => { + expect(() => + validateCreateEventInput({ + summary: 'Team Meeting', + start: { dateTime: '2024-01-15T10:00:00Z' }, + end: { dateTime: '2024-01-15T11:00:00Z' }, + attendees: ['not-an-email'], + }), + ).toThrow('Invalid email format'); + }); + }); + + describe('validateUpdateEventInput', () => { + it('accepts all-day date updates', () => { + expect(() => + validateUpdateEventInput({ + eventId: 'event123', + start: { date: '2024-01-15' }, + end: { date: '2024-01-16' }, + }), + ).not.toThrow(); + }); + + it('rejects empty start objects', () => { + const messages = getZodIssueMessages(() => + validateUpdateEventInput({ + eventId: 'event123', + start: {}, + }), + ); + + expect(messages).toContain( + 'start must have exactly one of "dateTime" or "date"', + ); + }); + + it('rejects invalid dateTime strings', () => { + expect(() => + validateUpdateEventInput({ + eventId: 'event123', + start: { dateTime: 'not-a-date' }, + }), + ).toThrow('Invalid ISO 8601 datetime format'); + }); + + it('rejects invalid calendar dates', () => { + expect(() => + validateUpdateEventInput({ + eventId: 'event123', + start: { date: '2024-02-30' }, + }), + ).toThrow('Invalid date format. Expected YYYY-MM-DD'); + }); + }); +}); diff --git a/workspace-server/src/index.ts b/workspace-server/src/index.ts index 59a01ce..e21916b 100644 --- a/workspace-server/src/index.ts +++ b/workspace-server/src/index.ts @@ -27,6 +27,27 @@ import { SCOPES } from './auth/scopes'; import { resolveFeatures } from './features/index'; // Shared schemas for calendar event tools +const eventDateInputSchema = (fieldName: string) => + z + .object({ + dateTime: z + .string() + .optional() + .describe( + 'Time in strict ISO 8601 format with seconds and timezone (e.g., 2024-01-15T10:30:00Z or 2024-01-15T10:30:00-05:00).', + ), + date: z + .string() + .optional() + .describe('Date in YYYY-MM-DD format. Use for all-day events.'), + }) + .refine( + ({ dateTime, date }) => Number(!!dateTime) + Number(!!date) === 1, + { + message: `${fieldName} must include exactly one of "dateTime" or "date"`, + }, + ); + const eventMeetAndAttachmentsSchema = { addGoogleMeet: z .boolean() @@ -55,7 +76,7 @@ const eventMeetAndAttachmentsSchema = { ) .optional() .describe( - 'Google Drive file attachments. IMPORTANT: Providing attachments fully REPLACES any existing attachments on the event (not appended).', + 'Google Drive file attachments. IMPORTANT: Providing attachments fully REPLACES any existing attachments on the event (not appended). On updates, pass an empty array to clear all attachments.', ), }; @@ -644,34 +665,8 @@ async function main() { .string() .optional() .describe('The description of the event.'), - start: z.object({ - dateTime: z - .string() - .optional() - .describe( - 'The start time in strict ISO 8601 format with seconds and timezone (e.g., 2024-01-15T10:30:00Z or 2024-01-15T10:30:00-05:00). Use for timed events.', - ), - date: z - .string() - .optional() - .describe( - 'The start date in YYYY-MM-DD format. Use for all-day events.', - ), - }), - end: z.object({ - dateTime: z - .string() - .optional() - .describe( - 'The end time in strict ISO 8601 format with seconds and timezone (e.g., 2024-01-15T11:30:00Z or 2024-01-15T11:30:00-05:00). Use for timed events.', - ), - date: z - .string() - .optional() - .describe( - 'The end date in YYYY-MM-DD format. Use for all-day events (exclusive, so use next day).', - ), - }), + start: eventDateInputSchema('start'), + end: eventDateInputSchema('end'), attendees: z .array(z.string()) .optional() @@ -867,7 +862,7 @@ async function main() { 'calendar.updateEvent', { description: - "Updates an existing event in a calendar. Supports adding Google Meet links and Google Drive file attachments. When addGoogleMeet is true, the Meet URL will be in the response's hangoutLink field. Attachments fully replace any existing attachments (not appended).", + "Updates an existing event in a calendar while preserving unspecified fields. Supports both timed events (`dateTime`) and all-day events (`date`), along with Google Meet links and Google Drive file attachments. When addGoogleMeet is true, the Meet URL will be in the response's hangoutLink field. Attachments fully replace any existing attachments (not appended), and an empty attachments array clears them.", inputSchema: { eventId: z.string().describe('The ID of the event to update.'), calendarId: z @@ -882,24 +877,8 @@ async function main() { .string() .optional() .describe('The new description of the event.'), - start: z - .object({ - dateTime: z - .string() - .describe( - 'The new start time in strict ISO 8601 format with seconds and timezone (e.g., 2024-01-15T10:30:00Z or 2024-01-15T10:30:00-05:00).', - ), - }) - .optional(), - end: z - .object({ - dateTime: z - .string() - .describe( - 'The new end time in strict ISO 8601 format with seconds and timezone (e.g., 2024-01-15T11:30:00Z or 2024-01-15T11:30:00-05:00).', - ), - }) - .optional(), + start: eventDateInputSchema('start').optional(), + end: eventDateInputSchema('end').optional(), attendees: z .array(z.string()) .optional() diff --git a/workspace-server/src/services/CalendarService.ts b/workspace-server/src/services/CalendarService.ts index e67f3bb..aa105fe 100644 --- a/workspace-server/src/services/CalendarService.ts +++ b/workspace-server/src/services/CalendarService.ts @@ -8,7 +8,11 @@ import crypto from 'node:crypto'; import { calendar_v3, google } from 'googleapis'; import { logToFile } from '../utils/logger'; import { gaxiosOptions } from '../utils/GaxiosConfig'; -import { iso8601DateTimeSchema, emailArraySchema } from '../utils/validation'; +import { iso8601DateTimeSchema } from '../utils/validation'; +import { + validateCreateEventInput, + validateUpdateEventInput, +} from './CalendarValidation'; import { z } from 'zod'; /** @@ -123,6 +127,7 @@ export class CalendarService { params: { conferenceDataVersion?: number; supportsAttachments?: boolean }, addGoogleMeet?: boolean, attachments?: EventAttachment[], + options?: { allowEmptyAttachments?: boolean }, ): void { if (addGoogleMeet) { event.conferenceData = { @@ -133,7 +138,10 @@ export class CalendarService { }; params.conferenceDataVersion = 1; } - if (attachments && attachments.length > 0) { + if ( + attachments && + (attachments.length > 0 || options?.allowEmptyAttachments) + ) { event.attachments = attachments.map((a) => ({ fileUrl: a.fileUrl, title: a.title, @@ -145,7 +153,11 @@ export class CalendarService { private createValidationErrorResponse(error: unknown) { const errorMessage = - error instanceof Error ? error.message : 'Validation failed'; + error instanceof z.ZodError + ? error.issues.map((issue) => issue.message).join('; ') + : error instanceof Error + ? error.message + : 'Validation failed'; let helpMessage = 'Please use strict ISO 8601 format with seconds and timezone. Examples: 2024-01-15T10:30:00Z (UTC) or 2024-01-15T10:30:00-05:00 (EST)'; @@ -260,122 +272,8 @@ export class CalendarService { const summary = input.summary ?? (eventType ? summaryDefaults[eventType] : undefined); - // Validate start: exactly one of dateTime or date must be provided - if ((!start.dateTime && !start.date) || (start.dateTime && start.date)) { - return this.createValidationErrorResponse( - new z.ZodError([ - { - code: 'custom', - message: - 'start must have exactly one of "dateTime" (for timed events) or "date" (for all-day events)', - path: ['start'], - }, - ]), - ); - } - - // Validate end: exactly one of dateTime or date must be provided - if ((!end.dateTime && !end.date) || (end.dateTime && end.date)) { - return this.createValidationErrorResponse( - new z.ZodError([ - { - code: 'custom', - message: - 'end must have exactly one of "dateTime" (for timed events) or "date" (for all-day events)', - path: ['end'], - }, - ]), - ); - } - - // Require summary for regular events - if ((!eventType || eventType === 'default') && !input.summary) { - return this.createValidationErrorResponse( - new z.ZodError([ - { - code: 'custom', - message: 'summary is required for regular events', - path: ['summary'], - }, - ]), - ); - } - - // Focus time and out-of-office cannot be all-day events (Google Calendar API constraint) - if ( - (eventType === 'focusTime' || eventType === 'outOfOffice') && - (start.date || end.date) - ) { - return this.createValidationErrorResponse( - new z.ZodError([ - { - code: 'custom', - message: `${eventType} events cannot be all-day events; use dateTime instead of date`, - path: ['start/end'], - }, - ]), - ); - } - - // workingLocationProperties is required when eventType is workingLocation - if (eventType === 'workingLocation' && !workingLocationProperties) { - return this.createValidationErrorResponse( - new z.ZodError([ - { - code: 'custom', - message: - 'workingLocationProperties is required when eventType is "workingLocation"', - path: ['workingLocationProperties'], - }, - ]), - ); - } - - // Validate working location sub-properties match the declared type - if (eventType === 'workingLocation' && workingLocationProperties) { - if ( - workingLocationProperties.type === 'officeLocation' && - !workingLocationProperties.officeLocation - ) { - return this.createValidationErrorResponse( - new z.ZodError([ - { - code: 'custom', - message: - 'officeLocation is required when workingLocationProperties.type is "officeLocation"', - path: ['workingLocationProperties', 'officeLocation'], - }, - ]), - ); - } - if ( - workingLocationProperties.type === 'customLocation' && - !workingLocationProperties.customLocation - ) { - return this.createValidationErrorResponse( - new z.ZodError([ - { - code: 'custom', - message: - 'customLocation is required when workingLocationProperties.type is "customLocation"', - path: ['workingLocationProperties', 'customLocation'], - }, - ]), - ); - } - } - - // Validate datetime formats (skip for date-only / all-day events) try { - if (start.dateTime) { - iso8601DateTimeSchema.parse(start.dateTime); - } - if (end.dateTime) { - iso8601DateTimeSchema.parse(end.dateTime); - } - if (attendees) { - emailArraySchema.parse(attendees); - } + validateCreateEventInput(input); } catch (error) { return this.createValidationErrorResponse(error); } @@ -479,6 +377,7 @@ export class CalendarService { insertParams, addGoogleMeet, attachments, + { allowEmptyAttachments: false }, ); const res = await calendar.events.insert(insertParams); @@ -672,45 +571,8 @@ export class CalendarService { attachments, } = input; - // Validate start/end: if provided, exactly one of dateTime or date - if (start) { - if ((start.dateTime && start.date) || (!start.dateTime && !start.date)) { - return this.createValidationErrorResponse( - new z.ZodError([ - { - code: 'custom', - message: 'start must have exactly one of "dateTime" or "date"', - path: ['start'], - }, - ]), - ); - } - } - if (end) { - if ((end.dateTime && end.date) || (!end.dateTime && !end.date)) { - return this.createValidationErrorResponse( - new z.ZodError([ - { - code: 'custom', - message: 'end must have exactly one of "dateTime" or "date"', - path: ['end'], - }, - ]), - ); - } - } - - // Validate datetime formats if provided try { - if (start?.dateTime !== undefined) { - iso8601DateTimeSchema.parse(start.dateTime); - } - if (end?.dateTime !== undefined) { - iso8601DateTimeSchema.parse(end.dateTime); - } - if (attendees) { - emailArraySchema.parse(attendees); - } + validateUpdateEventInput(input); } catch (error) { return this.createValidationErrorResponse(error); } @@ -723,8 +585,6 @@ export class CalendarService { try { const calendar = await this.getCalendar(); - - // Build request body with only the fields to update (patch semantics) const requestBody: calendar_v3.Schema$Event = {}; if (summary !== undefined) requestBody.summary = summary; if (description !== undefined) requestBody.description = description; @@ -733,7 +593,7 @@ export class CalendarService { if (attendees) requestBody.attendees = attendees.map((email) => ({ email })); - const updateParams: calendar_v3.Params$Resource$Events$Update = { + const updateParams: calendar_v3.Params$Resource$Events$Patch = { calendarId: finalCalendarId, eventId, requestBody, @@ -743,9 +603,10 @@ export class CalendarService { updateParams, addGoogleMeet, attachments, + { allowEmptyAttachments: true }, ); - const res = await calendar.events.update(updateParams); + const res = await calendar.events.patch(updateParams); logToFile(`Successfully updated event: ${res.data.id}`); return { diff --git a/workspace-server/src/services/CalendarValidation.ts b/workspace-server/src/services/CalendarValidation.ts new file mode 100644 index 0000000..413bffc --- /dev/null +++ b/workspace-server/src/services/CalendarValidation.ts @@ -0,0 +1,207 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { z } from 'zod'; +import { emailArraySchema, iso8601DateTimeSchema } from '../utils/validation'; +import type { + CalendarEventType, + CreateEventInput, + UpdateEventInput, +} from './CalendarService'; + +type EventDateInput = { + dateTime?: string | null; + date?: string | null; +}; + +type WorkingLocationValidationInput = { + type?: string | null; + officeLocation?: unknown; + customLocation?: unknown; +}; + +type CompleteEventValidationInput = { + summary?: string | null; + start: EventDateInput; + end: EventDateInput; + attendees?: string[]; + eventType?: CalendarEventType | null; + workingLocationProperties?: WorkingLocationValidationInput | null; +}; + +const isoDateSchema = z.string().refine( + (val) => { + if (!/^\d{4}-\d{2}-\d{2}$/.test(val)) return false; + const parsed = new Date(`${val}T00:00:00Z`); + if (Number.isNaN(parsed.getTime())) return false; + return parsed.toISOString().slice(0, 10) === val; + }, + { + message: 'Invalid date format. Expected YYYY-MM-DD', + }, +); + +function createIssue(path: (string | number)[], message: string): z.ZodError { + return new z.ZodError([ + { + code: 'custom', + message, + path, + }, + ]); +} + +function validateExclusiveDateField( + fieldName: 'start' | 'end', + fieldValue: EventDateInput, +): void { + const hasDateTime = !!fieldValue.dateTime; + const hasDate = !!fieldValue.date; + + if ((!hasDateTime && !hasDate) || (hasDateTime && hasDate)) { + const label = + fieldName === 'start' + ? 'start must have exactly one of "dateTime" (for timed events) or "date" (for all-day events)' + : 'end must have exactly one of "dateTime" (for timed events) or "date" (for all-day events)'; + + throw createIssue([fieldName], label); + } +} + +function validateOptionalExclusiveDateField( + fieldName: 'start' | 'end', + fieldValue?: EventDateInput, +): void { + if (!fieldValue) { + return; + } + + const hasDateTime = !!fieldValue.dateTime; + const hasDate = !!fieldValue.date; + + if ((!hasDateTime && !hasDate) || (hasDateTime && hasDate)) { + throw createIssue( + [fieldName], + `${fieldName} must have exactly one of "dateTime" or "date"`, + ); + } +} + +function validateDateFieldFormats(fieldName: 'start' | 'end', field: EventDateInput) { + if (field.dateTime) { + iso8601DateTimeSchema.parse(field.dateTime); + } + if (field.date) { + isoDateSchema.parse(field.date); + } +} + +function validateWorkingLocationProperties( + workingLocationProperties?: WorkingLocationValidationInput | null, +): void { + if (!workingLocationProperties) { + throw createIssue( + ['workingLocationProperties'], + 'workingLocationProperties is required when eventType is "workingLocation"', + ); + } + + if ( + workingLocationProperties.type === 'officeLocation' && + !workingLocationProperties.officeLocation + ) { + throw createIssue( + ['workingLocationProperties', 'officeLocation'], + 'officeLocation is required when workingLocationProperties.type is "officeLocation"', + ); + } + + if ( + workingLocationProperties.type === 'customLocation' && + !workingLocationProperties.customLocation + ) { + throw createIssue( + ['workingLocationProperties', 'customLocation'], + 'customLocation is required when workingLocationProperties.type is "customLocation"', + ); + } +} + +function addDays(date: string, days: number): string { + const parsed = new Date(`${date}T00:00:00Z`); + parsed.setUTCDate(parsed.getUTCDate() + days); + return parsed.toISOString().slice(0, 10); +} + +function validateWorkingLocationDuration(input: CompleteEventValidationInput): void { + if ( + input.eventType === 'workingLocation' && + input.start.date && + input.end.date && + addDays(input.start.date, 1) !== input.end.date + ) { + throw createIssue( + ['start', 'end'], + 'all-day workingLocation events must span exactly one day', + ); + } +} + +function validateCompleteEventInput(input: CompleteEventValidationInput): void { + validateExclusiveDateField('start', input.start); + validateExclusiveDateField('end', input.end); + validateDateFieldFormats('start', input.start); + validateDateFieldFormats('end', input.end); + + if ((!input.eventType || input.eventType === 'default') && !input.summary) { + throw createIssue(['summary'], 'summary is required for regular events'); + } + + if ( + (input.eventType === 'focusTime' || input.eventType === 'outOfOffice') && + (input.start.date || input.end.date) + ) { + throw createIssue( + ['start', 'end'], + `${input.eventType} events cannot be all-day events; use dateTime instead of date`, + ); + } + + if (input.eventType === 'workingLocation') { + validateWorkingLocationProperties(input.workingLocationProperties); + validateWorkingLocationDuration(input); + } + + if (input.attendees) { + emailArraySchema.parse(input.attendees); + } +} + +export function validateCreateEventInput(input: CreateEventInput): void { + validateCompleteEventInput({ + summary: input.summary, + start: input.start, + end: input.end, + attendees: input.attendees, + eventType: input.eventType, + workingLocationProperties: input.workingLocationProperties, + }); +} + +export function validateUpdateEventInput(input: UpdateEventInput): void { + validateOptionalExclusiveDateField('start', input.start); + validateOptionalExclusiveDateField('end', input.end); + + if (input.start) { + validateDateFieldFormats('start', input.start); + } + if (input.end) { + validateDateFieldFormats('end', input.end); + } + if (input.attendees) { + emailArraySchema.parse(input.attendees); + } +} From a92c505ac9edb21d946057fe7e85c0284f303e50 Mon Sep 17 00:00:00 2001 From: Kamil Galuszka Date: Tue, 7 Apr 2026 16:47:05 +0200 Subject: [PATCH 2/3] fix: address really great @allenhutchison code review feedback --- CONTRIBUTING.md | 9 +- .../services/CalendarService.test.ts | 98 +++++++++++++++++-- .../services/CalendarValidation.test.ts | 61 +++++++++++- workspace-server/src/index.ts | 9 +- .../src/services/CalendarService.ts | 53 ++++++++-- .../src/services/CalendarValidation.ts | 40 +++++--- 6 files changed, 229 insertions(+), 41 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 005668a..5a3506f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -83,8 +83,13 @@ ready for a formal review but is open for discussion and initial feedback. #### 4. Ensure All Checks Pass Before submitting your PR, ensure that all automated checks are passing by -running `npm run test && npm run lint`. This command runs all tests, linting, -and other style checks. +running: + +```bash +npm run test && npm run lint && npm run format:check && npx tsc --noEmit --project workspace-server +``` + +This command runs all tests, linting, formatting, and type checks. #### 5. Write Clear Commit Messages and a Good PR Description diff --git a/workspace-server/src/__tests__/services/CalendarService.test.ts b/workspace-server/src/__tests__/services/CalendarService.test.ts index d405cf4..0acbb9c 100644 --- a/workspace-server/src/__tests__/services/CalendarService.test.ts +++ b/workspace-server/src/__tests__/services/CalendarService.test.ts @@ -63,6 +63,7 @@ describe('CalendarService', () => { afterEach(() => { jest.restoreAllMocks(); + expect(mockCalendarAPI.events.update).not.toHaveBeenCalled(); }); describe('listCalendars', () => { @@ -168,6 +169,20 @@ describe('CalendarService', () => { expect(JSON.parse(result.content[0].text)).toEqual(mockCreatedEvent); }); + it('creates a single-day all-day workingLocation event', async () => { + mockCalendarAPI.events.insert.mockResolvedValue({ data: { id: 'wl1' } }); + await calendarService.createEvent({ + start: { date: '2024-01-15' }, + end: { date: '2024-01-16' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'homeOffice' }, + }); + const body = mockCalendarAPI.events.insert.mock.calls[0][0].requestBody; + expect(body.start).toEqual({ date: '2024-01-15' }); + expect(body.end).toEqual({ date: '2024-01-16' }); + expect(body.eventType).toBe('workingLocation'); + }); + it('should create a calendar event', async () => { const eventInput = { calendarId: 'primary', @@ -921,6 +936,41 @@ describe('CalendarService', () => { expect(parsedResult.error).toBe('Update failed'); }); + it('should surface structured Google API field errors on update', async () => { + mockCalendarAPI.events.patch.mockRejectedValue({ + response: { + data: { + error: { + message: 'Invalid Value', + code: 400, + errors: [ + { + location: 'start.dateTime', + reason: 'invalid', + message: 'Invalid start time', + }, + { + location: 'attendees', + reason: 'invalid', + message: 'Attendee email is invalid', + }, + ], + }, + }, + }, + }); + + const result = await calendarService.updateEvent({ + eventId: 'event123', + start: { dateTime: '2024-01-15T10:00:00Z' }, + }); + + const parsedResult = JSON.parse(result.content[0].text); + expect(parsedResult.error).toBe( + 'Invalid Value (code 400): start.dateTime invalid: Invalid start time; attendees invalid: Attendee email is invalid', + ); + }); + it('should only send fields that are provided', async () => { const updatedEvent = { id: 'event123', @@ -943,24 +993,56 @@ describe('CalendarService', () => { }); }); - it('should patch only changed fields for status events', async () => { + it.each([ + ['summary', { summary: 'X' }], + ['description', { description: 'X' }], + ['start', { start: { dateTime: '2024-01-15T10:00:00Z' } }], + ['end', { end: { dateTime: '2024-01-15T11:00:00Z' } }], + ['attendees', { attendees: ['a@b.com'] }], + ])( + '#313: patch body for %s update contains only that field', + async (field, patch) => { + mockCalendarAPI.events.patch.mockResolvedValue({ data: { id: 'e' } }); + await calendarService.updateEvent({ eventId: 'e', ...patch }); + const body = mockCalendarAPI.events.patch.mock.calls[0][0].requestBody; + expect(Object.keys(body)).toEqual([field]); + }, + ); + + it('should clear description when passed an empty string', async () => { mockCalendarAPI.events.patch.mockResolvedValue({ - data: { - id: 'focus123', - summary: 'Deep Work', + data: { id: 'event123', description: '' }, + }); + + await calendarService.updateEvent({ + eventId: 'event123', + description: '', + }); + + expect(mockCalendarAPI.events.patch).toHaveBeenCalledWith({ + calendarId: 'primary', + eventId: 'event123', + requestBody: { + description: '', }, }); + }); + + it('should clear attendees when passed an empty array', async () => { + mockCalendarAPI.events.patch.mockResolvedValue({ + data: { id: 'event123', attendees: [] }, + }); await calendarService.updateEvent({ - eventId: 'focus123', - summary: 'Deep Work', + eventId: 'event123', + attendees: [], }); expect(mockCalendarAPI.events.patch).toHaveBeenCalledWith({ calendarId: 'primary', - eventId: 'focus123', + eventId: 'event123', requestBody: { - summary: 'Deep Work', + attendees: [], }, }); }); diff --git a/workspace-server/src/__tests__/services/CalendarValidation.test.ts b/workspace-server/src/__tests__/services/CalendarValidation.test.ts index 7805f38..cc4f5f9 100644 --- a/workspace-server/src/__tests__/services/CalendarValidation.test.ts +++ b/workspace-server/src/__tests__/services/CalendarValidation.test.ts @@ -44,9 +44,7 @@ describe('CalendarValidation', () => { eventType: 'workingLocation', workingLocationProperties: { type: 'homeOffice' }, }), - ).toThrow( - 'all-day workingLocation events must span exactly one day', - ); + ).toThrow('all-day workingLocation events must span exactly one day'); }); it('accepts a leap-day working location event that spans one day', () => { @@ -60,6 +58,61 @@ describe('CalendarValidation', () => { ).not.toThrow(); }); + it('accepts a year-boundary working location event', () => { + expect(() => + validateCreateEventInput({ + start: { date: '2024-12-31' }, + end: { date: '2025-01-01' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'homeOffice' }, + }), + ).not.toThrow(); + }); + + it('accepts a month-boundary working location event', () => { + expect(() => + validateCreateEventInput({ + start: { date: '2024-01-31' }, + end: { date: '2024-02-01' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'homeOffice' }, + }), + ).not.toThrow(); + }); + + it('accepts a non-leap Feb working location event', () => { + expect(() => + validateCreateEventInput({ + start: { date: '2023-02-28' }, + end: { date: '2023-03-01' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'homeOffice' }, + }), + ).not.toThrow(); + }); + + it('rejects a same-day working location event', () => { + expect(() => + validateCreateEventInput({ + start: { date: '2024-01-15' }, + end: { date: '2024-01-15' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'homeOffice' }, + }), + ).toThrow('all-day workingLocation events must span exactly one day'); + }); + + it('rejects a reversed-date working location event', () => { + expect(() => + validateCreateEventInput({ + start: { date: '2024-01-16' }, + end: { date: '2024-01-15' }, + eventType: 'workingLocation', + workingLocationProperties: { type: 'homeOffice' }, + }), + ).toThrow('end.date must be on or after start.date'); + }); + it('rejects a regular event without a summary', () => { expect(() => validateCreateEventInput({ @@ -126,7 +179,7 @@ describe('CalendarValidation', () => { ); expect(messages).toContain( - 'start must have exactly one of "dateTime" or "date"', + 'start must have exactly one of "dateTime" (for timed events) or "date" (for all-day events)', ); }); diff --git a/workspace-server/src/index.ts b/workspace-server/src/index.ts index e21916b..dd5f56c 100644 --- a/workspace-server/src/index.ts +++ b/workspace-server/src/index.ts @@ -41,12 +41,9 @@ const eventDateInputSchema = (fieldName: string) => .optional() .describe('Date in YYYY-MM-DD format. Use for all-day events.'), }) - .refine( - ({ dateTime, date }) => Number(!!dateTime) + Number(!!date) === 1, - { - message: `${fieldName} must include exactly one of "dateTime" or "date"`, - }, - ); + .refine(({ dateTime, date }) => Number(!!dateTime) + Number(!!date) === 1, { + message: `${fieldName} must have exactly one of "dateTime" (for timed events) or "date" (for all-day events)`, + }); const eventMeetAndAttachmentsSchema = { addGoogleMeet: z diff --git a/workspace-server/src/services/CalendarService.ts b/workspace-server/src/services/CalendarService.ts index aa105fe..386c7ac 100644 --- a/workspace-server/src/services/CalendarService.ts +++ b/workspace-server/src/services/CalendarService.ts @@ -154,7 +154,13 @@ export class CalendarService { private createValidationErrorResponse(error: unknown) { const errorMessage = error instanceof z.ZodError - ? error.issues.map((issue) => issue.message).join('; ') + ? error.issues + .map((issue) => + issue.path.length + ? `${issue.path.join('.')}: ${issue.message}` + : issue.message, + ) + .join('; ') : error instanceof Error ? error.message : 'Validation failed'; @@ -185,6 +191,43 @@ export class CalendarService { }; } + private extractErrorMessage(error: unknown): string { + const details = ( + error as { + response?: { + data?: { + error?: { + message?: string; + code?: number; + errors?: Array<{ + location?: string; + reason?: string; + message?: string; + }>; + }; + }; + }; + } + )?.response?.data?.error; + + if (details) { + let message = `${details.message} (code ${details.code})`; + if (details.errors?.length) { + const fieldErrors = details.errors + .map( + (e) => + [e.location, e.reason].filter(Boolean).join(' ') + + `: ${e.message}`, + ) + .join('; '); + message += `: ${fieldErrors}`; + } + return message; + } + + return error instanceof Error ? error.message : String(error); + } + private async getCalendar(): Promise { logToFile('Getting authenticated client for calendar...'); const auth = await this.authManager.getAuthenticatedClient(); @@ -391,8 +434,7 @@ export class CalendarService { ], }; } catch (error) { - const errorMessage = - error instanceof Error ? error.message : String(error); + const errorMessage = this.extractErrorMessage(error); logToFile(`Error during calendar.createEvent: ${errorMessage}`); return { content: [ @@ -590,7 +632,7 @@ export class CalendarService { if (description !== undefined) requestBody.description = description; if (start) requestBody.start = start; if (end) requestBody.end = end; - if (attendees) + if (attendees !== undefined) requestBody.attendees = attendees.map((email) => ({ email })); const updateParams: calendar_v3.Params$Resource$Events$Patch = { @@ -618,8 +660,7 @@ export class CalendarService { ], }; } catch (error) { - const errorMessage = - error instanceof Error ? error.message : String(error); + const errorMessage = this.extractErrorMessage(error); logToFile(`Error during calendar.updateEvent: ${errorMessage}`); return { content: [ diff --git a/workspace-server/src/services/CalendarValidation.ts b/workspace-server/src/services/CalendarValidation.ts index 413bffc..3ae74ba 100644 --- a/workspace-server/src/services/CalendarValidation.ts +++ b/workspace-server/src/services/CalendarValidation.ts @@ -62,12 +62,10 @@ function validateExclusiveDateField( const hasDate = !!fieldValue.date; if ((!hasDateTime && !hasDate) || (hasDateTime && hasDate)) { - const label = - fieldName === 'start' - ? 'start must have exactly one of "dateTime" (for timed events) or "date" (for all-day events)' - : 'end must have exactly one of "dateTime" (for timed events) or "date" (for all-day events)'; - - throw createIssue([fieldName], label); + throw createIssue( + [fieldName], + `${fieldName} must have exactly one of "dateTime" (for timed events) or "date" (for all-day events)`, + ); } } @@ -85,12 +83,15 @@ function validateOptionalExclusiveDateField( if ((!hasDateTime && !hasDate) || (hasDateTime && hasDate)) { throw createIssue( [fieldName], - `${fieldName} must have exactly one of "dateTime" or "date"`, + `${fieldName} must have exactly one of "dateTime" (for timed events) or "date" (for all-day events)`, ); } } -function validateDateFieldFormats(fieldName: 'start' | 'end', field: EventDateInput) { +function validateDateFieldFormats( + fieldName: 'start' | 'end', + field: EventDateInput, +) { if (field.dateTime) { iso8601DateTimeSchema.parse(field.dateTime); } @@ -136,17 +137,26 @@ function addDays(date: string, days: number): string { return parsed.toISOString().slice(0, 10); } -function validateWorkingLocationDuration(input: CompleteEventValidationInput): void { +function validateWorkingLocationDuration( + input: CompleteEventValidationInput, +): void { if ( input.eventType === 'workingLocation' && input.start.date && - input.end.date && - addDays(input.start.date, 1) !== input.end.date + input.end.date ) { - throw createIssue( - ['start', 'end'], - 'all-day workingLocation events must span exactly one day', - ); + if (input.end.date < input.start.date) { + throw createIssue( + ['start', 'end'], + 'end.date must be on or after start.date', + ); + } + if (addDays(input.start.date, 1) !== input.end.date) { + throw createIssue( + ['start', 'end'], + 'all-day workingLocation events must span exactly one day', + ); + } } } From 463f92e43db3eb5c491cdaa41e3a20edf38d4566 Mon Sep 17 00:00:00 2001 From: Kamil Galuszka Date: Tue, 7 Apr 2026 17:06:11 +0200 Subject: [PATCH 3/3] fix: improve error support based on Google API documentation --- .../src/services/CalendarService.ts | 36 ++++++++++++++----- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/workspace-server/src/services/CalendarService.ts b/workspace-server/src/services/CalendarService.ts index 386c7ac..33c0b2c 100644 --- a/workspace-server/src/services/CalendarService.ts +++ b/workspace-server/src/services/CalendarService.ts @@ -200,9 +200,11 @@ export class CalendarService { message?: string; code?: number; errors?: Array<{ - location?: string; + domain?: string; reason?: string; message?: string; + location?: string; + locationType?: string; }>; }; }; @@ -211,18 +213,34 @@ export class CalendarService { )?.response?.data?.error; if (details) { - let message = `${details.message} (code ${details.code})`; + const topLevelMessage = details.message ?? 'Unknown Error'; + const code = details.code ? ` (code ${details.code})` : ''; + if (details.errors?.length) { const fieldErrors = details.errors - .map( - (e) => - [e.location, e.reason].filter(Boolean).join(' ') + - `: ${e.message}`, - ) + .map((e) => { + const context = [e.domain, e.locationType, e.location] + .filter(Boolean) + .join('.'); + const identity = [context, e.reason].filter(Boolean).join(' '); + return identity ? `${identity}: ${e.message}` : e.message; + }) .join('; '); - message += `: ${fieldErrors}`; + + // If the top-level message is just a generic summary of the first field error, + // or if they are identical, just show the field errors to avoid stutter. + if ( + details.errors.length === 1 && + (topLevelMessage === details.errors[0].message || + topLevelMessage.includes(details.errors[0].message ?? '')) + ) { + return `${fieldErrors}${code}`; + } + + return `${topLevelMessage}${code}: ${fieldErrors}`; } - return message; + + return `${topLevelMessage}${code}`; } return error instanceof Error ? error.message : String(error);