diff --git a/ex_app/lib/all_tools/assignments.py b/ex_app/lib/all_tools/assignments.py index f1e6578..9fe80e5 100644 --- a/ex_app/lib/all_tools/assignments.py +++ b/ex_app/lib/all_tools/assignments.py @@ -25,12 +25,12 @@ async def create_scheduled_task(title: str, prompt: str, recurrence_rule: str, t :param title: A title for the Scheduled Task, e.g. "Transcribe audio files" -- This is only for the user's reference and has no effect on the execution of the Scheduled Task. :param prompt: The instructions for the agent carrying out the Scheduled Task (Do not tell it to create a scheduled task here, as this is the instruction that runs as the scheduled task already. So instead of "Create a scheduled task to do X", just pass "Do X" here) :param recurrence_rule: An RRule compliant with RFC 5545 that defines the recurrence rule for the Scheduled Task. For example "FREQ=DAILY;INTERVAL=1" to run the Scheduled Task every day, an empty string as the recurrence_rule means the task does not repeat. - :param starts_at: A date time string in ISO 8601 format that defines when the Scheduled Task should start. For example "2025-01-01T09:00:00Z". If not provided, the Scheduled Task will start immediately. + :param starts_at: A date time string in ISO 8601 format in the user's local time that defines when the Scheduled Task should start. For example "2025-01-01T09:00:00". It is interpreted in the given timezone. If not provided, the Scheduled Task will start immediately. :param timezone: Timezone (e.g., 'America/New_York'). Defaults to the user's profile timezone; used to interpret starts_at. Must be a valid IANA timezone name when supplied. :return: """ - tz = get_timezone(timezone or await user_timezone_async(nc)) + tz = get_timezone(timezone) if timezone is not None else await user_timezone_async(nc) await nc.ocs('POST', f'/ocs/v2.php/apps/assistant/assignments', json={ 'title': title, 'prompt': prompt, @@ -60,16 +60,20 @@ async def update_scheduled_task(id: int, prompt: None|str = None, recurrence_rul :param id: The ID of the Scheduled Task to update, you can obtain this from the list_scheduled_tasks tool :param prompt: The instructions for the AI carrying out the Scheduled Task. Pass `None` to leave this unchanged. (Do not tell the other AI to create a scheduled task here, as this is the instruction that runs as the scheduled task already. So instead of "Create a scheduled task to do X", just pass "Do X" here) :param recurrence_rule: An RRule compliant with RFC 5545 that defines the recurrence rule for the Scheduled Task. For example "FREQ=DAILY;INTERVAL=1" to run the Scheduled Task every day. An empty string means, it does not repeat. Pass `None` to leave this unchanged. - :param timezone: Timezone (e.g., 'America/New_York'). Defaults to the user's profile timezone; used to interpret starts_at. Must be a valid IANA timezone name when supplied. - :param starts_at: A date time string in ISO 8601 format that defines when the Scheduled Task should start. For example "2025-01-01T09:00:00Z". If not provided, the Scheduled Task will start immediately. Pass `None` to leave this unchanged. + :param timezone: A new timezone for the Scheduled Task (e.g., 'America/New_York'), also used to interpret starts_at. Must be a valid IANA timezone name. Pass `None` to leave this unchanged. + :param starts_at: A date time string in ISO 8601 format in the user's local time that defines when the Scheduled Task should start. For example "2025-01-01T09:00:00". It is interpreted in the given timezone, or the user's profile timezone if none is given. Pass `None` to leave this unchanged. :return: """ - tz = get_timezone(timezone or await user_timezone_async(nc)) + # only send a timezone when one was given explicitly, so the stored one is left unchanged otherwise + tz = get_timezone(timezone) if timezone is not None else None + starts_at_ts = None + if starts_at is not None: + starts_at_ts = starts_at_timestamp(starts_at, tz or await user_timezone_async(nc)) return await nc.ocs('PATCH', f'/ocs/v2.php/apps/assistant/assignments/{id}', json={ 'prompt': prompt, 'recurrence': recurrence_rule, - 'startsAt': starts_at_timestamp(starts_at, tz) if starts_at is not None else None, + 'startsAt': starts_at_ts, 'timezone': tz }) diff --git a/ex_app/lib/all_tools/calendar.py b/ex_app/lib/all_tools/calendar.py index f5fb168..cb0eb60 100644 --- a/ex_app/lib/all_tools/calendar.py +++ b/ex_app/lib/all_tools/calendar.py @@ -402,6 +402,9 @@ def update_task_sync(calendar_name: str, task_uid: str, title: Optional[str] = N calendars = principal.calendars() calendar = {cal.name: cal for cal in calendars}[calendar_name] + # resolve the timezone before the loop, so an invalid or missing one is reported instead of swallowed below + tz = pytz.timezone(get_timezone(timezone or user_timezone(ncSync))) if due_date else None + todos = calendar.todos() for todo in todos: # Parse the todo data using ics library @@ -426,8 +429,7 @@ def update_task_sync(calendar_name: str, task_uid: str, title: Optional[str] = N else: due_datetime = parsed_date - timezone = timezone or user_timezone(ncSync) - due_datetime = pytz.timezone(get_timezone(timezone)).localize(due_datetime) + due_datetime = tz.localize(due_datetime) ics_todo.due = due_datetime diff --git a/ex_app/lib/all_tools/lib/tz.py b/ex_app/lib/all_tools/lib/tz.py index 38f0f62..9cc2006 100644 --- a/ex_app/lib/all_tools/lib/tz.py +++ b/ex_app/lib/all_tools/lib/tz.py @@ -33,8 +33,9 @@ async def user_timezone_async(nc) -> str | None: return None -def starts_at_timestamp(starts_at: str, timezone: str) -> int: +def starts_at_timestamp(starts_at: str, timezone: str | None) -> int: parsed = datetime.fromisoformat(starts_at.replace("Z", "+00:00")) if parsed.tzinfo is None or parsed.utcoffset() is None: - parsed = pytz.timezone(timezone).localize(parsed) + # only naive values need a timezone to be interpreted + parsed = pytz.timezone(get_timezone(timezone)).localize(parsed) return int(parsed.timestamp()) diff --git a/tests/test_timezone_resolution.py b/tests/test_timezone_resolution.py new file mode 100644 index 0000000..da4c802 --- /dev/null +++ b/tests/test_timezone_resolution.py @@ -0,0 +1,184 @@ +# SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors +# SPDX-License-Identifier: AGPL-3.0-or-later +"""Timezone resolution in the scheduled task and calendar task tools. + +Run standalone: + + PYTHONPATH=. python tests/test_timezone_resolution.py + +or via pytest when available. The network layer is stubbed with fakes. +""" +import asyncio +import inspect +import os +import sys + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from ex_app.lib.all_tools import assignments # noqa: E402 +from ex_app.lib.all_tools import calendar as cal # noqa: E402 +from ex_app.lib.all_tools.lib.tz import starts_at_timestamp, user_timezone_async # noqa: E402 + +_user_counter = 0 + + +class _FakeAsyncNc: + """Records assistant OCS calls; serves /cloud/user with the given profile timezone.""" + + def __init__(self, profile_timezone): + global _user_counter + _user_counter += 1 + # unique user per instance, so user_timezone_async's per-user cache does not leak between tests + self._user_id = f"tz-test-{_user_counter}" + self.profile_timezone = profile_timezone + self.calls = [] + + @property + def user(self): + async def _u(): + return self._user_id + + return _u() + + async def ocs(self, method, path, json=None): + if path == '/ocs/v2.php/cloud/user': + return {'timezone': self.profile_timezone} + self.calls.append((method, path, json)) + return True + + +def _assignment_tool(nc, name): + tools = {t.name: t for t in asyncio.run(assignments.get_tools(nc))} + return tools[name].coroutine + + +def test_update_without_timezone_leaves_stored_timezone_unchanged(): + nc = _FakeAsyncNc("Europe/Berlin") + asyncio.run(_assignment_tool(nc, "update_scheduled_task")(id=1, prompt="Do Y")) + (_, _, payload), = nc.calls + assert payload['timezone'] is None, payload + assert payload['startsAt'] is None, payload + + +def test_update_without_any_timezone_still_works(): + nc = _FakeAsyncNc(None) + asyncio.run(_assignment_tool(nc, "update_scheduled_task")(id=1, prompt="Do Y")) + assert nc.calls[0][2]['prompt'] == "Do Y" + + +def test_update_naive_starts_at_uses_profile_timezone(): + nc = _FakeAsyncNc("Europe/Berlin") + asyncio.run(_assignment_tool(nc, "update_scheduled_task")(id=1, starts_at="2026-10-21T09:00:00")) + payload = nc.calls[0][2] + assert payload['startsAt'] == starts_at_timestamp("2026-10-21T07:00:00Z", None), payload + assert payload['timezone'] is None, payload + + +def test_update_explicit_timezone_is_sent(): + nc = _FakeAsyncNc("Europe/Berlin") + asyncio.run(_assignment_tool(nc, "update_scheduled_task")(id=1, timezone="Asia/Tokyo")) + assert nc.calls[0][2]['timezone'] == "Asia/Tokyo" + + +def test_create_without_any_timezone_still_works(): + nc = _FakeAsyncNc(None) + asyncio.run(_assignment_tool(nc, "create_scheduled_task")(title="T", prompt="Do X", recurrence_rule="FREQ=DAILY")) + payload = nc.calls[0][2] + assert payload['timezone'] is None, payload + + +def test_create_defaults_to_profile_timezone(): + nc = _FakeAsyncNc("Europe/Berlin") + asyncio.run(_assignment_tool(nc, "create_scheduled_task")(title="T", prompt="Do X", recurrence_rule="FREQ=DAILY")) + assert nc.calls[0][2]['timezone'] == "Europe/Berlin" + + +def test_create_naive_starts_at_without_any_timezone_raises(): + nc = _FakeAsyncNc(None) + try: + asyncio.run(_assignment_tool(nc, "create_scheduled_task")(title="T", prompt="Do X", recurrence_rule="", starts_at="2026-10-21T09:00:00")) + except ValueError: + return + raise AssertionError("expected ValueError for a naive starts_at without a timezone") + + +def test_create_invalid_timezone_raises(): + nc = _FakeAsyncNc("Europe/Berlin") + try: + asyncio.run(_assignment_tool(nc, "create_scheduled_task")(title="T", prompt="Do X", recurrence_rule="", timezone="Not/AZone")) + except ValueError: + return + raise AssertionError("expected ValueError for invalid timezone") + + +def test_starts_at_with_offset_needs_no_timezone(): + assert starts_at_timestamp("2026-10-21T09:00:00+02:00", None) == starts_at_timestamp("2026-10-21T07:00:00Z", None) + + +class _FakeCalendar: + name = "Personal" + + def todos(self): + return [] + + +class _FakePrincipal: + def calendars(self): + return [_FakeCalendar()] + + +class _FakeCal: + def principal(self): + return _FakePrincipal() + + +class _FakeSyncNc: + cal = _FakeCal() + + def set_user(self, user): + pass + + @property + def user(self): + async def _u(): + return "tz-test-calendar" + + return _u() + + def ocs(self, method, path): + return {'timezone': "Europe/Berlin"} + + +def _update_task_sync(): + original = cal.NextcloudApp + cal.NextcloudApp = _FakeSyncNc + try: + tools = {t.name: t for t in asyncio.run(cal.get_tools(_FakeSyncNc()))} + finally: + cal.NextcloudApp = original + return inspect.getclosurevars(tools["update_task"].coroutine).nonlocals["update_task_sync"] + + +def test_update_task_invalid_timezone_raises(): + try: + _update_task_sync()("Personal", "uid", due_date="2026-11-01", timezone="Not/AZone") + except ValueError: + return + raise AssertionError("expected ValueError for invalid timezone, not a swallowed False") + + +def test_update_task_without_due_date_ignores_timezone(): + assert _update_task_sync()("Personal", "uid", title="x", timezone="Not/AZone") is False + + +if __name__ == "__main__": + failed = 0 + for name, fn in sorted(globals().items()): + if name.startswith("test_") and callable(fn): + try: + fn() + print(f"PASS {name}") + except Exception as exc: + failed += 1 + print(f"FAIL {name}: {type(exc).__name__}: {exc}") + sys.exit(1 if failed else 0)