Google calendar branch #126
Conversation
Resolved multiple issues that were causing internal server errors (500)
in the Calendar API replica:
1. Import endpoint organizer handling
- Fixed duplicate kwargs error when importing events with organizer field
- Events now correctly preserve custom organizer email from imports
- Organizer defaults to creator when not specified
2. Watch channel expiration parsing
- Added parse_watch_expiration() to handle multiple formats:
* Integer milliseconds since epoch
* String milliseconds (1739627400000)
* ISO 8601 date strings (2026-02-15T14:30:00Z)
- Applied to all watch endpoints: events, calendarList, ACL, settings
3. Channel expiration storage
- Changed Channel.expiration from Integer to BigInteger
- Prevents PostgreSQL integer overflow for millisecond timestamps
- Timestamps like 1893456000000 (year 2030) now store correctly
4. Attendees validation
- Added email field validation for attendees in create/update
- Returns proper 400 error with field location instead of 500 KeyError
- Validates both POST and PATCH operations
These fixes address 129 of the 500 errors identified in benchmark testing,
with remaining errors being cascading failures that are now resolved.
Files changed:
- backend/src/services/calendar/api/methods.py
- backend/src/services/calendar/database/operations.py
- backend/src/services/calendar/database/schema.py
WalkthroughCentralizes watch expiration parsing/validation, updates Channel.expiration to BigInteger and adds payload flag, refactors watch endpoints to use the new parser, adds per-attendee email validation and datetime normalization in event operations, and updates tests and .gitignore rules. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/services/calendar/database/schema.py (1)
633-639:⚠️ Potential issue | 🟠 MajorCreate a migration to change
calendar_channels.expirationfrom Integer to BigInteger.The
expirationcolumn is currently defined as Integer in the database (from initial migration 2ac6c433442a) but the schema now declares it as BigInteger. Without a migration to alter this column type, there will be a runtime schema mismatch. Thepayloadcolumn already exists in the initial migration and requires no changes.
🤖 Fix all issues with AI agents
In @.gitignore:
- Around line 244-247: The negated patterns !.claude/CLAUDE.md and
!.claude/settings.json won’t work because the parent directory is ignored by an
earlier `.claude` entry and by `.claude/**`; update the ignore rules so the
directory itself is unignored (add `!.claude/` before the specific file
exceptions) or remove the earlier `.claude` ignore, and keep the `.claude/**`
rule only if you still explicitly unignore the directory with `!.claude/` so the
negations for `!.claude/CLAUDE.md` and `!.claude/settings.json` take effect.
In `@backend/src/services/calendar/api/methods.py`:
- Around line 1099-1109: The Channel instantiation uses an unnecessary f-string
for resource_uri (resource_uri=f"/users/me/calendarList") which triggers Ruff
F541; change the f-string to a plain string literal
(resource_uri="/users/me/calendarList") inside the Channel(...) call where
resource_id is created by generate_resource_id() and expiration is set via
parse_watch_expiration(body.get("expiration")), leaving all other fields
(id=channel_id, type=channel_type, address=address, token=body.get("token"))
unchanged.
- Around line 108-168: parse_watch_expiration currently calls parse_rfc3339 and
uses dt.timestamp(), which treats naive datetimes as local time; update
parse_watch_expiration to detect a naive datetime returned by parse_rfc3339
(dt.tzinfo is None) and replace its tzinfo with datetime.timezone.utc (or call
dt.replace(tzinfo=timezone.utc)) before calling timestamp(), so ISO strings
without zone are normalized to UTC prior to converting to milliseconds; keep
existing int conversion and ValidationError handling intact.
In `@backend/src/services/calendar/database/operations.py`:
- Around line 1043-1057: The current _normalize_dt(dt) maps None to datetime.min
causing all-day events (which have start_datetime=None but start_date set) to
sort to the top; change the sorting to use the event's start_date as a fallback
when start_datetime is None by adjusting the sort key (or _normalize_dt) to
accept the event (or both dt and the event) and, if dt is None, convert
event.start_date to a timezone-aware datetime at the start of day (timezone.utc)
and return that; keep the existing behavior for updated_at sorting (only use
updated_at conversion) and continue using e.id as the tiebreaker.
In `@examples/calendar/testsuites/calendar_bench.json`:
- Around line 5669-5671: The test JSON contains a redundant/mismatched assertion
where the "start" field is being used to assert an end date value; specifically
remove or correct the duplicate/misplaced assertion so that only the
"end.dateTime" assertion contains "2018-08-23" (or change the "start" assertion
to assert the actual start date if that was intended). Update the assertions
referencing "start" and "end.dateTime" in the calendar_bench JSON so they each
validate the appropriate date field consistently.
🧹 Nitpick comments (1)
examples/calendar/testsuites/calendar_bench.json (1)
701-714: Extensive ignore list may mask unintended changes.This ignore list excludes most event fields (attendees, color_id, description, end, recurrence, reminders, start, status, summary, transparency, visibility), leaving only
locationchanges to be verified. While this focuses the assertion on the intended change, it could mask regressions in other fields.Consider documenting why such an extensive ignore list is necessary here, or verify this is intentional for this specific test scenario.
When parse_rfc3339 returns a naive datetime (no timezone info), explicitly set tzinfo to UTC before calling timestamp(). This ensures consistent behavior regardless of server timezone and matches the pattern used in format_rfc3339. Co-authored-by: Hubert <hubert-marek@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/src/services/calendar/api/methods.py`:
- Around line 137-139: The current isinstance(expiration, int) check accepts
booleans because bool subclasses int; update the check in the
expiration-handling block (the one that currently reads "if
isinstance(expiration, int): return expiration") to explicitly exclude bools so
True/False are not treated as ints (e.g., use "if isinstance(expiration, int)
and not isinstance(expiration, bool): return expiration" or "if type(expiration)
is int: return expiration") so boolean values fall through to the existing
ValidationError path around line 170.
| # If already an int, return as-is | ||
| if isinstance(expiration, int): | ||
| return expiration |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n backend/src/services/calendar/api/methods.py | sed -n '120,160p'Repository: hubert-marek/agent-diff
Length of output: 1592
🏁 Script executed:
cat -n backend/src/services/calendar/api/methods.py | sed -n '160,180p'Repository: hubert-marek/agent-diff
Length of output: 838
Reject boolean expirations explicitly.
bool is a subclass of int in Python, so passing True or False will be accepted and stored as 1 or 0 milliseconds (representing 1970-01-01 00:00:00.001 UTC and Unix epoch respectively). These are not valid expiration times for calendar watch subscriptions. Tighten the isinstance check to reject booleans explicitly so they fall through to the existing ValidationError at line 170.
🔧 Suggested fix
- if isinstance(expiration, int):
+ if isinstance(expiration, int) and not isinstance(expiration, bool):
return expiration🤖 Prompt for AI Agents
In `@backend/src/services/calendar/api/methods.py` around lines 137 - 139, The
current isinstance(expiration, int) check accepts booleans because bool
subclasses int; update the check in the expiration-handling block (the one that
currently reads "if isinstance(expiration, int): return expiration") to
explicitly exclude bools so True/False are not treated as ints (e.g., use "if
isinstance(expiration, int) and not isinstance(expiration, bool): return
expiration" or "if type(expiration) is int: return expiration") so boolean
values fall through to the existing ValidationError path around line 170.
1 participant
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.