Skip to content

feat(calendar_sync): read a user's calendar over the Google and Graph APIs, not just an ICS feed - #1636

Open
DmitriyG228 wants to merge 3 commits into
mainfrom
feat/calendar-oauth-providers
Open

DmitriyG228 wants to merge 3 commits into
mainfrom
feat/calendar-oauth-providers

Conversation

@DmitriyG228

@DmitriyG228 DmitriyG228 commented Sep 6, 2026

Copy link
Copy Markdown
Member

Draft. The seam only — nothing calls these yet. Opened as the findable artifact for a pushed
branch rather than as a review request; mark ready on the founder's word.

COMMITMENTS IN THIS PR

None. No published terms, no dates, no rights, no spend, no external promise. Additive code
behind no route and no flag: two new pure functions, their tests, and the module's own docs.
The commitments in the wider calendar-OAuth effort (the privacy-policy amendment, entering Google's
verification programme) live in ~/dev/biz/drafts/2026-09-06-calendar-oauth-review-clock.md and are
gated separately — none of them is decided here.

Why

Connecting a calendar today means pasting a secret ICS address. Google Workspace hides that field
under the default admin policy and Outlook requires publishing a calendar, so the setup step for the
buyer we care about is "file a ticket with IT". Six open issues also exist purely because we
dereference a user-supplied URL and parse RFC 5545 ourselves (#1182, #1231, #1232, #1316, #991,
#1249), and a polled feed has a 5-minute latency floor with no push.

What this is

calendar_sync was already split so the pipeline is reader-agnostic: sync_user owns one row per
UID, adoption by link, occurrence disposition and retirement, and it consumes a plain dict. So a
calendar API is a new reader, not a new pipeline.

parse_ics(text)              -> {"events": [...], "cancelled_uids": [...]}
events_from_google(items)    -> {"events": [...], "cancelled_uids": [...]}   NEW
events_from_microsoft(items) -> {"events": [...], "cancelled_uids": [...]}   NEW

Pure — no network, no tokens, no clock of their own. Recurrence expansion is the provider's job
(singleEvents=true, /calendarView); we then apply the same rule parse_ics applies: group by the
series-stable id, keep the earliest occurrence in the window, because two scheduled rows on one
native id violate uq_meeting_active_user_platform_native.

The decision worth reviewing

Keyed on iCalUID / iCalUId, not on the per-occurrence event['id']. That is the same value
the ICS feed carries, so a calendar reconnected from ICS to OAuth adopts its existing rows.
Keying on the instance id would have re-imported every existing user's whole calendar as duplicates
on the day they switch. Executed rather than asserted —
test_a_calendar_reconnected_over_oauth_adopts_its_own_ics_rows runs sync_user over a feed and
then over the API payload for the same meeting and asserts created == 0.

Two deliberate divergences from the ICS reader

ICS reader These readers Why
Event snapshot copies every VEVENT property bounded allowlist #1213 item 3 — "no test that the VEVENT snapshot is redacted"; an API returns far more per event than a feed (extended properties, attachments, ACL hints)
Naive timestamp n/a resolved against its stated zone, else UTC never the server's local zone — that is #1316 and backlog R-B10. Wrong by a known offset beats wrong by wherever the pod runs

Attendees fold onto the ICS PARTSTAT vocabulary (accepted / declined / tentative /
needs-action) so nothing downstream has to learn two dialects; rooms and equipment are dropped on
both sides, matching service._attendees.

Tests

tests/test_calendar_providers.py, 28 cases. The parity ones are the point: same PlannedEvent keys
as parse_ics, same attendee vocabulary, same one-row-per-uid rule, same "a link-less event still
imports" rule, and three that drive the real sync_user over the in-memory store.

meeting-api: 1450 passed, 5 skipped   (28 new)

Run locally against the repo venv, no containers.

Not in this PR

No OAuth flow, no token storage, no route, no config-record change, no push subscriptions, no UI.
Nothing imports these functions yet. Token storage in particular has a hard prerequisite: #876
per-user secrets sit plaintext in users.data JSONB, and a leaked refresh token is a different
category of problem from a leaked ICS URL. Envelope encryption lands before or with the connect flow.

Second commit: the I/O half

provider_io.py — fetch a window of events, refresh a token. fetch_ics dereferences a
user-supplied URL and must ride the SSRF-pinned transport; these talk to two fixed hosts, so the
risk inverts and what matters is that stored user input never escapes into the URL:

  • calendar ids are path-escaped against a constant base (tested with ../../tokeninfo?x=)
  • redirects off, so an Authorization header cannot be moved to another host
  • Graph's @odata.nextLink is host-checked before it is followed with a token attached

Same error contract as fetch_ics(value, human_reason), never raises, because one user's dead
network or revoked grant must not stall every other user's calendar in the same sweep. The reason is
what the calendar panel shows, so it reads "reconnect the calendar", not "401".

An invalid_grant refresh is treated as terminal, not transient. Retrying a dead grant on a loop
hammers the identity provider with a credential that will never work again.

Scopes are calendar.events.readonly + calendar.calendarlist.readonly and a test pins that they
stay read-only. We never write to a calendar, and a wider scope is a bigger consent prompt, a slower
Google review, and more to lose if a token leaks.

Pagination is exhaustive but bounded at 20 pages: past ~5000 events in a 14-day window this is not a
person's calendar any more.

17 further tests, offline through a stub client. meeting-api: 1467 passed, 5 skipped.

Note on value-fsm

It failed once on this branch (test_mock_silence_left_alonePOST /bots 500), then passed on
re-run against the identical head
. Not caused by this change: meeting_api.calendar_sync and
meeting_api.__main__ both import cleanly with the new eager import, and the suite is green locally.
Flagging it because a gate that flakes on a mock scenario is worth someone's attention on its own.

Collision check


Contribution rights

  • Independent: I created this contribution, or otherwise have the right to submit it
    under Apache-2.0, and it is not owned or controlled by an employer, client, or other entity.
  • Employer/client authorization required: an employer, client, or other entity owns or
    may control this contribution. I am requesting Vexa's private corporate-authorization process.
  • Unsure: I need a private rights review before merge.

Every commit must also carry the contributor's own DCO Signed-off-by line. Selecting the
independent path means no individual CLA is required.

Section restored by an agent because the PR body was written without the repo template. Nothing
is ticked and nothing will be
— the template says an agent may explain the choices but must not
select one, so this is left for a human. The commits also lack Signed-off-by, which is the same
kind of certification; sign them with:
git rebase --signoff origin/main && git push --force-with-lease

…ents as the ICS feed

The ICS feed is one way to learn a user's calendar, not the concept. sync_user already owns every
hard part — one row per UID, adoption by link, occurrence disposition, retirement — and consumes a
plain dict, so a calendar API is a new READER, not a new pipeline.

  parse_ics(text)             -> {events, cancelled_uids}
  events_from_google(items)   -> {events, cancelled_uids}   (new)
  events_from_microsoft(items)-> {events, cancelled_uids}   (new)

Both are pure: no network, no tokens, no clock of their own. Recurrence expansion is the
provider's job (singleEvents=true / calendarView), after which the same load-bearing rule applies
— group by the series-stable id, keep the earliest occurrence in the window.

Keyed on iCalUID / iCalUId, deliberately: that is the value the feed carried, so a calendar
reconnected from ICS to OAuth ADOPTS its existing rows. Executed, not asserted —
test_a_calendar_reconnected_over_oauth_adopts_its_own_ics_rows asserts created == 0. Keying on the
per-occurrence event id would have re-imported every user's whole calendar as duplicates.

Two deliberate divergences from the ICS reader, both closing known residuals:
  - bounded per-event snapshot instead of copying arbitrary provider properties (#1213 item 3)
  - a naive provider timestamp resolves against its stated zone, else UTC — never the server's
    local zone (#1316, backlog R-B10)

Attendees fold onto the ICS PARTSTAT vocabulary so nothing downstream learns two dialects.

No wiring yet: nothing calls these, no OAuth flow, no token storage, no route. This is the seam
only. [meeting-api 1450 passed / 5 skipped; 28 new]
README + __init__ describe the reader/pipeline split and what each API reader's caller must send
(singleEvents+showDeleted for Google, Prefer: outlook.timezone="UTC" for Graph).
@DmitriyG228
DmitriyG228 marked this pull request as ready for review September 6, 2026 19:11
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

🃏 Merge card — #1636

check what it needs
Value missing state: value-signed (the value sign-off)
Diff maintainer self-review — @DmitriyG228 holds the commit bit (no separate non-author review required)

Not mergeable yet — every row above must be accepted before merge (choke point 1). Fill in what's ❌ above, then this clears automatically.

How a PR reaches merge: the merge bar.

@DmitriyG228 DmitriyG228 added the docs: none PR touches a product surface but needs no docs change (D6c waiver, give reason) label Sep 6, 2026
@DmitriyG228

Copy link
Copy Markdown
Member Author

docs: none reason (D6c): this PR adds two pure functions that nothing imports yet — no route, no config field, no flag, no user-visible behaviour. The module's own README.md is updated in the diff. User-facing docs (docs/docs/how-to/calendar-sync.mdx, which today documents the secret-ICS-address flow) change when the connect flow ships, not here — documenting an OAuth option that does not exist would be the wrong kind of current.

…refresh the token

The I/O half of the API readers. fetch_ics dereferences a user-supplied URL and must ride the
SSRF-pinned transport; these talk to two fixed hosts, so the risk inverts — what must never happen
is stored user input escaping into the URL. Calendar ids are path-escaped against a constant base,
redirects are off so an Authorization header cannot be moved to another host, and Graph's
@odata.nextLink is host-checked before it is followed with a token attached.

Error contract copied from fetch_ics: (value, human_reason), never raises. One user's dead network
or revoked grant must not stall every other user's calendar in the same sweep, and the reason is
what the calendar panel shows, so it says 'reconnect the calendar' rather than '401'.

An invalid_grant refresh is terminal, not transient — revoked, password changed, or expired through
disuse. Retrying it on a loop hammers the identity provider with a credential that will never work
again, so it surfaces as reconnect.

Scopes are read-only and narrow (calendar.events.readonly + calendar.calendarlist.readonly): a
wider scope is a bigger consent prompt, a slower Google review, and more to lose if a token leaks.
We never write to a calendar. A test pins that.

Pagination is exhaustive but bounded at 20 pages — past 5000 events in a 14-day window it is not a
person's calendar any more.

Tokens are arguments, never state: nothing here reads a database or a secret store, which keeps it
offline-testable and keeps decrypt-then-use in one auditable place upstream.

[meeting-api 1467 passed / 5 skipped; 17 new]
@DmitriyG228 DmitriyG228 changed the title feat(calendar_sync): Google and Graph readers emit the same PlannedEvents as the ICS feed feat(calendar_sync): read a user's calendar over the Google and Graph APIs, not just an ICS feed Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs: none PR touches a product surface but needs no docs change (D6c waiver, give reason)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant