diff --git a/src/fabledassistant/services/tools/calendar.py b/src/fabledassistant/services/tools/calendar.py index 90828e2..12fc43c 100644 --- a/src/fabledassistant/services/tools/calendar.py +++ b/src/fabledassistant/services/tools/calendar.py @@ -2,7 +2,8 @@ from __future__ import annotations -from datetime import datetime, timezone +import re +from datetime import date as _date, datetime, time as _time, timezone from fabledassistant.services.events import ( create_event as events_create_event, @@ -17,10 +18,49 @@ from fabledassistant.services.tools._registry import tool from fabledassistant.services.tz import get_user_tz +_DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") +_TIME_RE = re.compile(r"^\d{2}:\d{2}(:\d{2})?$") + + +async def _combine_local_in_user_tz( + user_id: int, date_str: str, time_str: str | None +) -> datetime: + """Build a UTC datetime from separate date and time strings. + + The whole point of this helper: a `YYYY-MM-DD` string carries no TZ + metadata that a model could mis-tag, so the calendar day cannot drift + across the local→UTC boundary. The wall-clock `HH:MM` likewise has no + TZ; we attach the user's local zone explicitly via ``datetime.combine`` + and then convert to UTC for storage. + + Strict shape validation rejects anything that isn't a bare date or a + bare time — no `2026-05-01Z`, no `08:00 UTC` slipping through. + """ + if not _DATE_RE.match(date_str): + raise ValueError( + f"start_date / end_date must be YYYY-MM-DD with no timezone; got {date_str!r}" + ) + d = _date.fromisoformat(date_str) + if time_str is None or time_str == "": + t = _time(0, 0) + else: + if not _TIME_RE.match(time_str): + raise ValueError( + f"start_time / end_time must be HH:MM (or HH:MM:SS), no timezone; got {time_str!r}" + ) + t = _time.fromisoformat(time_str) + user_tz = await get_user_tz(user_id) + local = datetime.combine(d, t, tzinfo=user_tz) + return local.astimezone(timezone.utc) + + async def _parse_datetime_in_user_tz( user_id: int, value: str ) -> tuple[datetime, bool]: - """Parse a date/datetime string from the model into a UTC-aware datetime. + """Legacy single-string parser. Kept as a fallback when the model + emits the older `start` / `end` shape; new calls should use + `start_date`+`start_time` (and `end_date`+`end_time`) which sidestep + the TZ-tagging foot-gun this parser is vulnerable to. Naive inputs are interpreted in the **user's local timezone** and then converted to UTC for storage. Never default to UTC for naive inputs — @@ -38,14 +78,54 @@ async def _parse_datetime_in_user_tz( return dt.astimezone(timezone.utc), was_date_only +async def _resolve_event_start( + user_id: int, args: dict +) -> tuple[datetime, bool]: + """Resolve the start datetime from either the new split fields + (`start_date` + optional `start_time`) or the legacy combined `start`. + Returns ``(utc_datetime, was_date_only)``.""" + if "start_date" in args and args["start_date"]: + date_str = args["start_date"] + time_str = args.get("start_time") or None + return await _combine_local_in_user_tz(user_id, date_str, time_str), time_str is None + if "start" in args and args["start"]: + return await _parse_datetime_in_user_tz(user_id, args["start"]) + raise ValueError("Either start_date or start is required") + + +async def _resolve_event_end( + user_id: int, args: dict +) -> datetime | None: + """Resolve the end datetime from either the new split fields or the + legacy combined `end`. Returns ``None`` when no end fields are set.""" + if "end_date" in args and args["end_date"]: + date_str = args["end_date"] + time_str = args.get("end_time") or None + return await _combine_local_in_user_tz(user_id, date_str, time_str) + if "end" in args and args["end"]: + dt, _ = await _parse_datetime_in_user_tz(user_id, args["end"]) + return dt + return None + + @tool( name="create_event", - description="Create a calendar event for the user. Use this when the user asks to schedule, add, or create a meeting, appointment, or event. Pass dates and datetimes in the user's local time — a bare date like '2026-09-30' is interpreted as that day in the user's configured timezone.", + description=( + "Create a calendar event for the user. Use this when the user asks " + "to schedule, add, or create a meeting, appointment, or event. " + "Always pass `start_date` (YYYY-MM-DD) and `start_time` (HH:MM) as " + "separate fields in the user's local time — never combine them and " + "never include a timezone suffix. The server attaches the user's " + "configured timezone. Omit `start_time` (or set `all_day=true`) " + "for all-day events like birthdays or holidays." + ), parameters={ "title": {"type": "string", "description": "A descriptive event title"}, - "start": {"type": "string", "description": "Start date (YYYY-MM-DD) or datetime (YYYY-MM-DDTHH:MM) in the user's local time. Include a timezone offset only if the user explicitly mentions one."}, - "end": {"type": "string", "description": "Optional end date or datetime in the user's local time"}, - "duration": {"type": "integer", "description": "Optional duration in minutes (default 60, ignored if end is set or all_day is true)"}, + "start_date": {"type": "string", "description": "Start calendar date as YYYY-MM-DD in the user's local time. No timezone suffix."}, + "start_time": {"type": "string", "description": "Start wall-clock time as HH:MM (24-hour). Omit for all-day events. No timezone suffix."}, + "end_date": {"type": "string", "description": "Optional end calendar date as YYYY-MM-DD."}, + "end_time": {"type": "string", "description": "Optional end wall-clock time as HH:MM."}, + "duration": {"type": "integer", "description": "Optional duration in minutes (default 60, ignored if end_date/end_time is set or all_day is true)"}, "description": {"type": "string", "description": "Optional event description"}, "location": {"type": "string", "description": "Optional event location"}, "color": {"type": "string", "description": "Optional hex color for the event (e.g. '#6366f1')"}, @@ -55,30 +135,31 @@ async def _parse_datetime_in_user_tz( "attendees": {"type": "array", "items": {"type": "string"}, "description": "Optional list of attendee email addresses"}, "calendar_name": {"type": "string", "description": "Optional calendar name to create the event in. Falls back to default calendar."}, "project": {"type": "string", "description": "Optional project name to associate this event with"}, + # Legacy combined fields kept for backward compatibility with saved + # tool-call payloads in conversation history. New calls should use + # start_date + start_time. Hidden from typical model output via the + # description above; still accepted by the resolver as a fallback. + "start": {"type": "string", "description": "[Deprecated] Combined start datetime — prefer start_date + start_time."}, + "end": {"type": "string", "description": "[Deprecated] Combined end datetime — prefer end_date + end_time."}, }, - required=["title", "start"], + required=["title"], ) async def create_event_tool(*, user_id, arguments, **_ctx): - start_str = arguments["start"] - end_str = arguments.get("end") all_day = arguments.get("all_day", False) try: - # Naive dates/datetimes are interpreted in the user's local - # timezone, not UTC. Storing UTC for naive inputs caused all-day - # events to land on the previous day for negative-offset users. - start_dt, start_was_date_only = await _parse_datetime_in_user_tz( - user_id, start_str - ) - except (ValueError, TypeError): - return {"success": False, "error": f"Invalid start datetime: {start_str!r}"} + start_dt, start_was_date_only = await _resolve_event_start(user_id, arguments) + except ValueError as exc: + return {"success": False, "error": str(exc)} + except TypeError as exc: + return {"success": False, "error": f"Invalid start: {exc}"} if start_was_date_only: all_day = True - end_dt = None - if end_str: - try: - end_dt, _ = await _parse_datetime_in_user_tz(user_id, end_str) - except (ValueError, TypeError): - return {"success": False, "error": f"Invalid end datetime: {end_str!r}"} + try: + end_dt = await _resolve_event_end(user_id, arguments) + except ValueError as exc: + return {"success": False, "error": str(exc)} + except TypeError as exc: + return {"success": False, "error": f"Invalid end: {exc}"} project_id = None project_name = arguments.get("project") if project_name: @@ -168,18 +249,29 @@ async def search_events_tool(*, user_id, arguments, **_ctx): @tool( name="update_event", - description="Update an existing calendar event. Use this when the user asks to change, move, reschedule, or modify an event.", + description=( + "Update an existing calendar event. Use this when the user asks to " + "change, move, reschedule, or modify an event. Pass `start_date` " + "(YYYY-MM-DD) and `start_time` (HH:MM) as separate fields in the " + "user's local time when rescheduling — never combine them, never " + "include a timezone suffix." + ), parameters={ "query": {"type": "string", "description": "Search term to find the event to update (matches against title)"}, "title": {"type": "string", "description": "New title for the event"}, - "start": {"type": "string", "description": "New start datetime in ISO 8601 format"}, - "end": {"type": "string", "description": "New end datetime in ISO 8601 format"}, + "start_date": {"type": "string", "description": "New start calendar date as YYYY-MM-DD in the user's local time. No timezone suffix."}, + "start_time": {"type": "string", "description": "New start wall-clock time as HH:MM. No timezone suffix."}, + "end_date": {"type": "string", "description": "New end calendar date as YYYY-MM-DD."}, + "end_time": {"type": "string", "description": "New end wall-clock time as HH:MM."}, "all_day": {"type": "boolean", "description": "Whether the event is all-day"}, "description": {"type": "string", "description": "New event description"}, "location": {"type": "string", "description": "New event location"}, "color": {"type": "string", "description": "New hex color for the event (e.g. '#6366f1')"}, "recurrence": {"type": "string", "description": "New iCalendar RRULE"}, "reminder_minutes": {"type": "integer", "description": "Reminder N minutes before the event. Pass 0 to remove an existing reminder."}, + # Legacy combined fields kept for backcompat — see create_event. + "start": {"type": "string", "description": "[Deprecated] Combined start datetime — prefer start_date + start_time."}, + "end": {"type": "string", "description": "[Deprecated] Combined end datetime — prefer end_date + end_time."}, }, required=["query"], ) @@ -198,16 +290,20 @@ async def update_event_tool(*, user_id, arguments, **_ctx): if "reminder_minutes" in arguments: rm = arguments["reminder_minutes"] fields["reminder_minutes"] = None if rm == 0 else rm - for dt_field, key in (("start_dt", "start"), ("end_dt", "end")): - val = arguments.get(key) - if val: - try: - # Naive datetimes are user-local, not UTC — see - # ``_parse_datetime_in_user_tz`` docstring. - dt, _ = await _parse_datetime_in_user_tz(user_id, val) - fields[dt_field] = dt - except (ValueError, TypeError): - return {"success": False, "error": f"Invalid datetime for {key}: {val!r}"} + # Resolve start: split fields preferred, legacy `start` as fallback. + if arguments.get("start_date") or arguments.get("start"): + try: + start_dt, _ = await _resolve_event_start(user_id, arguments) + except (ValueError, TypeError) as exc: + return {"success": False, "error": f"Invalid start: {exc}"} + fields["start_dt"] = start_dt + if arguments.get("end_date") or arguments.get("end"): + try: + end_dt = await _resolve_event_end(user_id, arguments) + except (ValueError, TypeError) as exc: + return {"success": False, "error": f"Invalid end: {exc}"} + if end_dt is not None: + fields["end_dt"] = end_dt updated = await events_update_event(user_id=user_id, event_id=event_to_update.id, **fields) if updated is None: return {"success": False, "error": "Event not found or update failed."} diff --git a/tests/test_calendar_tool_tz.py b/tests/test_calendar_tool_tz.py index bab715f..6d711fa 100644 --- a/tests/test_calendar_tool_tz.py +++ b/tests/test_calendar_tool_tz.py @@ -132,3 +132,266 @@ async def test_list_events_bare_date_range_covers_local_day(): assert (df.year, df.month, df.day, df.hour) == (2026, 9, 30, 4) # 2026-09-30 23:59:59 NY (EDT) = 03:59:59 UTC next day assert (dt.year, dt.month, dt.day, dt.hour) == (2026, 10, 1, 3) + + +# ── Split date/time field tests (durable shape) ────────────────────────────── +# +# These exercise the start_date + start_time path that the model is now +# steered toward. The split-field shape is structurally immune to the +# class of bugs where a model emits a TZ-tagged combined datetime that +# the parser correctly honors but lands on the wrong calendar day for +# negative-offset users. + + +@pytest.mark.asyncio +async def test_create_event_split_fields_friday_8am_no_drift_eastern(): + """The reported bug: 'next Friday at 8am' for a NY user must land on + 2026-05-01 08:00 NY, never 04-30 19:00. With split fields the model + can't TZ-tag the date string, so the calendar day is fixed.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + result = await create_event_tool( + user_id=1, + arguments={ + "title": "Meeting", + "start_date": "2026-05-01", + "start_time": "08:00", + }, + ) + + assert result["success"] is True + utc = captured["start_dt"].astimezone(timezone.utc) + # 08:00 NY (EDT, UTC-4) on 2026-05-01 = 12:00 UTC same day + assert (utc.year, utc.month, utc.day, utc.hour, utc.minute) == (2026, 5, 1, 12, 0) + assert captured["all_day"] is False + + +@pytest.mark.asyncio +async def test_create_event_split_fields_no_drift_pacific(): + """Same scenario for a UTC-8 user — the calendar day must be 5/1 + regardless of how big the offset gets.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/Los_Angeles")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + await create_event_tool( + user_id=1, + arguments={ + "title": "Standup", + "start_date": "2026-05-01", + "start_time": "08:00", + }, + ) + + utc = captured["start_dt"].astimezone(timezone.utc) + # 08:00 LA (PDT, UTC-7) = 15:00 UTC same day + assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 5, 1, 15) + + +@pytest.mark.asyncio +async def test_create_event_split_fields_no_drift_positive_offset(): + """A positive-offset user (Tokyo, UTC+9) — 08:00 Tokyo on 2026-05-01 + is 23:00 UTC on 2026-04-30, but the local calendar day must still be + stored as 2026-05-01 from the user's perspective.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("Asia/Tokyo")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + await create_event_tool( + user_id=1, + arguments={ + "title": "Sync", + "start_date": "2026-05-01", + "start_time": "08:00", + }, + ) + + utc = captured["start_dt"].astimezone(timezone.utc) + # 08:00 Tokyo (UTC+9) = 23:00 UTC previous day; the round-trip back + # to Tokyo TZ recovers 2026-05-01 08:00. + assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 4, 30, 23) + tokyo = captured["start_dt"].astimezone(__import__("zoneinfo").ZoneInfo("Asia/Tokyo")) + assert (tokyo.year, tokyo.month, tokyo.day, tokyo.hour) == (2026, 5, 1, 8) + + +@pytest.mark.asyncio +async def test_create_event_split_date_only_is_all_day(): + """Omitting start_time means the event is all-day; cache row uses + local midnight.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + await create_event_tool( + user_id=1, + arguments={"title": "Birthday", "start_date": "2026-09-30"}, + ) + + assert captured["all_day"] is True + utc = captured["start_dt"].astimezone(timezone.utc) + assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 9, 30, 4) + + +@pytest.mark.asyncio +async def test_create_event_split_rejects_tz_suffix_in_date(): + """The whole point of split fields: a TZ suffix on the date string + must be rejected, not silently honored.""" + from fabledassistant.services.tools.calendar import create_event_tool + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ): + result = await create_event_tool( + user_id=1, + arguments={"title": "x", "start_date": "2026-05-01Z", "start_time": "08:00"}, + ) + + assert result["success"] is False + assert "YYYY-MM-DD" in result["error"] + + +@pytest.mark.asyncio +async def test_create_event_split_rejects_tz_suffix_in_time(): + """A TZ suffix on the time string must also be rejected.""" + from fabledassistant.services.tools.calendar import create_event_tool + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ): + result = await create_event_tool( + user_id=1, + arguments={"title": "x", "start_date": "2026-05-01", "start_time": "08:00 UTC"}, + ) + + assert result["success"] is False + assert "HH:MM" in result["error"] + + +@pytest.mark.asyncio +async def test_create_event_legacy_combined_start_still_works(): + """Backcompat: saved tool-call payloads using the old `start` field + must still produce the same UTC datetime as before.""" + from fabledassistant.services.tools.calendar import create_event_tool + + captured = {} + + async def fake_create_event(**kwargs): + captured.update(kwargs) + event = AsyncMock() + event.to_dict.return_value = {"id": 1} + return event + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.events_create_event", + side_effect=fake_create_event, + ): + result = await create_event_tool( + user_id=1, + arguments={"title": "Legacy", "start": "2026-05-01T08:00"}, + ) + + assert result["success"] is True + utc = captured["start_dt"].astimezone(timezone.utc) + assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 5, 1, 12) + + +@pytest.mark.asyncio +async def test_update_event_split_fields_reschedule_no_drift(): + """update_event with split fields must drift no calendar day either.""" + from fabledassistant.services.tools.calendar import update_event_tool + + captured = {} + + async def fake_find(*, user_id, query): + ev = AsyncMock() + ev.id = 99 + ev.title = "Coffee" + return [ev] + + async def fake_update(*, user_id, event_id, **fields): + captured.update(fields) + ev = AsyncMock() + ev.to_dict.return_value = {"id": event_id, **fields} + return ev + + with patch( + "fabledassistant.services.tools.calendar.get_user_tz", + AsyncMock(return_value=__import__("zoneinfo").ZoneInfo("America/New_York")), + ), patch( + "fabledassistant.services.tools.calendar.find_events_by_query", + side_effect=fake_find, + ), patch( + "fabledassistant.services.tools.calendar.events_update_event", + side_effect=fake_update, + ): + result = await update_event_tool( + user_id=1, + arguments={ + "query": "Coffee", + "start_date": "2026-05-01", + "start_time": "08:00", + }, + ) + + assert result["success"] is True + utc = captured["start_dt"].astimezone(timezone.utc) + assert (utc.year, utc.month, utc.day, utc.hour) == (2026, 5, 1, 12)