Skip to content

fix(desktop): connect Calendar with Google sign-in, and stop swallowing connector errors - #12262

Open
aryanorastar wants to merge 9 commits into
BasedHardware:mainfrom
aryanorastar:fix/connector-error-copy
Open

fix(desktop): connect Calendar with Google sign-in, and stop swallowing connector errors#12262
aryanorastar wants to merge 9 commits into
BasedHardware:mainfrom
aryanorastar:fix/connector-error-copy

Conversation

@aryanorastar

@aryanorastar aryanorastar commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Two users reported Gmail and Google Calendar failing to connect on desktop, with this and nothing else:

Couldn't connect to Calendar. Try again.

There are two defects behind that screenshot. One is why the connect fails; the other is why the user can't tell.

1. The connect fails because the desktop authenticates by reading browser cookies

Desktop Calendar was the only client still decrypting Google auth cookies out of a Chromium profile. Issue #10459 documents three failure modes with no recoverable action:

  • No Chromium browser installed. A user signed into Google in Safari or Firefox gets no_browser and an immediate failure.
  • Cookies the reader will not decrypt. BrowserGoogleSession.swift:79 deliberately skips versioned blobs it has no key for, so a signed-in user reads as not signed in.
  • Declined Keychain "Safe Storage" prompt. The browser is silently dropped from the scan.

In every one of those, "Try again" is not advice — nothing about the machine has changed between attempts.

The backend already owns a real Google Calendar OAuth grant, the same one the mobile app connects. This routes the desktop through it:

  • readEvents prefers the account's server-held grant, cookies second. That is one edit in the function all six call sites share — connector, Settings, both onboarding paths, chat provider, automation bridge — rather than six call-site patches.
  • Connect Calendar opens Google's consent screen and polls for the grant when both sources are exhausted, mirroring the existing connectX flow, instead of dead-ending.
  • verifyConnection reports connected off a real grant-backed fetch, so status stops telling grant-holders they need to sign in.
  • The event payload gains location, description and all_day. The cookie reader read these straight from Google; without them every memory built from an event silently loses detail. max_results goes 100 → 500 for the connector's one-pass year of history (Google's own list cap is 2500).

What this PR deliberately does not do

The cookie reader stays. Retiring it means deleting ~800 lines of Swift, its embedded Python, the Settings account picker and their tests — the unreviewable migration AGENTS.md says not to fold into a bug fix. It stays tracked on #10459. Ordering is grant-first with cookies as fallback, so nobody the scraper serves today stops working.

Gmail is not migrated. gmail.readonly is a Google restricted scope, and integrations_registry.py withholds it from the consent request until verification and CASA are granted:

requesting it before verification + CASA are granted makes Google show every user an "unverified app" screen and blocks sign-in

Routing Gmail through the grant today would report connected: false forever. Gmail keeps the cookie reader, and gets honest errors from part 2. Unblocking it is Google's decision, not a code change.

2. The user can't tell, because the one message that would have helped is exactly one character too long

The connector sheet stores an operation's user-facing message as a plain String and re-sanitizes it at display time (AppsPage.swift:1817). The sanitizer's last rule was:

return text.hasSuffix(".") && text.count < 120

CalendarReaderError.notSignedIn's description is exactly 120 characters. It fails that ceiling by one and renders as the generic fallback — which for .integration("Calendar") is literally "Couldn't connect to Calendar. Try again.", the reported string. It is also the failure the cookie reader produces most often, and the one that most needs its next step shown.

Measuring every connector error string against the old predicate:

LOST  len=120  cal.notSignedIn      ← exactly 120, ceiling is < 120
LOST  len= 65  cal.pythonNotFound
LOST  len= 47  gm.cookieDecrypt
LOST  len= 31  gm.network
LOST  len= 65  gm.pythonNotFound
KEEP  ...10 others

The other four are lost for lacking a trailing period — the interpolated ones end in a detail fragment.

Sentence shape does not separate copy this app wrote from raw system text. The raw-error fingerprints that already ran above it do, so they are now the whole decision. The Error Domain= / nsurlerror pair is replaced by errordomain, which also catches the bare NSPOSIXErrorDomain spellings the old checks missed — so this is strictly stronger at hiding raw text while no longer eating curated copy.

Verification

The regression test reproduces the reported string from unmodified production code. Reverting only shouldPreserveCuratedCopy and re-running:

Executed 7 tests, with 5 failures
error: ... testEveryGoogleConnectorErrorSurvivesDisplaySanitization :
  XCTAssertEqual failed: ("Couldn't connect to Calendar. Try again.")

Five failures, one per lost message. With the fix, 7/7 pass. The test drives off the production CalendarReaderError / GmailReaderError enums rather than copied literals, so editing a message into a shape the sanitizer drops fails here instead of shipping.

Check Result
dev-feedback.py --once swift 'UserFacingErrorPresentationTests|GoogleCalendarGrantEventTests' Executed 9 tests, 0 failures
python3 -m pytest tests/unit/test_google_calendar_event_response.py 4 passed
./scripts/swift-test-suites.sh (full suite — result below)
make preflight (result below)

Endpoints are live in prod today. Built and launched as named bundle omi-calendar-oauth against https://api.omi.me/ (confirmed via omi-ctl health), then probed the three routes the desktop now calls:

v1/integrations/google_calendar                401
v1/integrations/google_calendar/oauth-url      401
v1/calendar/google/events?max_results=1        401
v1/integrations/google_calendar/nope           404   ← control

401 means the route exists and wants auth; the control under the same prefix 404s. So the desktop's new calls resolve against production as-is. Only the added event fields and the raised max_results need a backend deploy.

Not verified, stated plainly

The authenticated grant read was not exercised end to end. The named bundle came up signed out (AUTH_LISTENER: No saved session) and signing into an account is not an action I take on someone's behalf, so the live grant → events → memories path still needs one manual run by a signed-in maintainer. Everything up to the auth boundary is covered above.

The reporters' own failure was not reproduced locally. This Mac's Arc profile holds live SAPISID / __Secure-1PSID cookies in a version the reader decrypts, and Calendar connect works here — the failure needs a machine with no Chromium, or cookies under a scheme the reader skips. The cookie-path diagnosis is taken from #10459's evidence, not from my own measurement. The display defect is deterministic given the string, and is proven above.

Product invariants

INV-INT-1 (integrations harness over heuristics). This change moves toward the invariant on three counts: it deletes a heuristic rather than adding one; the OAuth setup flow ends in a functional probe rather than a latch; and verifyConnection's new branch performs a real one-event fetch, so "Connected" continues to mean verified now.

Failure-Class: none

Review in cubic

The connector sheet stores an operation's user-facing message as a plain
String and re-sanitizes it at display time. The sanitizer's last rule was
`text.hasSuffix(".") && text.count < 120` — a guess at "is this curated"
layered on top of the raw-error fingerprints that already ran.

CalendarReaderError.notSignedIn's description is exactly 120 characters,
so it failed that ceiling by one and rendered as the generic fallback:
"Couldn't connect to Calendar. Try again." That is the single most common
Google connect failure and the one that most needs its next step shown.
Four more messages were lost the same way, for lacking a trailing period:
both pythonNotFound cases and Gmail's network and decrypt errors.

Sentence shape does not separate copy this app wrote from raw system
text. The fingerprints do, so they are now the whole decision — and the
`Error Domain=` / `nsurlerror` pair is replaced by `errordomain`, which
also catches bare NSPOSIXErrorDomain spellings the old checks missed.

Verified:
- desktop/macos/scripts/dev-feedback.py --once swift
  'UserFacingErrorPresentationTests' → Executed 7 tests, 0 failures.
- Against the unmodified predicate the same suite reports 5 failures, the
  first of them literally
  `XCTAssertEqual failed: ("Couldn't connect to Calendar. Try again.")` —
  the reported string, reproduced from production code.

Failure-Class: none
Desktop Calendar was the only client still authenticating by decrypting
Google auth cookies out of a Chromium profile. That mechanism has three
failure modes with no recoverable action, all in issue BasedHardware#10459: no
Chromium browser installed (Safari or Firefox users), cookies re-encrypted
under a scheme the reader will not open, and a declined browser Keychain
"Safe Storage" prompt. Each one dead-ends at "Try again", where nothing
about the machine has changed and trying again cannot help.

The backend already owns a real Google Calendar OAuth grant — the same one
the mobile app connects. This routes the desktop through it:

- readEvents prefers the account's server-held grant and keeps cookies as
  the fallback, so the fix reaches all six call sites (connector, Settings,
  onboarding, chat provider, automation bridge) through the one function
  they share rather than six call-site patches.
- Connect Calendar now opens Google's consent screen and polls for the
  grant when both sources are exhausted, mirroring the existing connectX
  flow, instead of dead-ending.
- verifyConnection reports connected off a real grant-backed fetch, so
  status stops claiming "needs sign in" to accounts that hold a grant.
- The event payload gains location, description and all_day, which the
  cookie reader read straight from Google; without them every memory built
  from an event silently loses detail. max_results ceiling goes 100 → 500
  for the connector's one-pass year of history (Google's own cap is 2500).

The cookie reader stays. Retiring it means deleting ~800 lines of Swift,
its embedded Python, the Settings account picker and their tests — the
unreviewable migration AGENTS.md says not to fold into a bug fix. It stays
tracked on BasedHardware#10459.

Gmail is deliberately not migrated: gmail.readonly is a Google *restricted*
scope and integrations_registry.py withholds it from the consent request
until verification and CASA are granted, so routing Gmail through the grant
would report connected: false forever. It keeps the cookie reader and, from
the previous commit, honest errors.

Verified:
- backend: python3 -m pytest tests/unit/test_google_calendar_event_response.py
  → 4 passed.
- desktop: dev-feedback.py --once swift
  'UserFacingErrorPresentationTests|GoogleCalendarGrantEventTests'
  → Executed 9 tests, 0 failures.
- Built and launched as the named bundle omi-calendar-oauth against
  https://api.omi.me/ (confirmed via omi-ctl health).
- All three endpoints the desktop now calls are live in prod today:
  v1/integrations/google_calendar, .../oauth-url and
  v1/calendar/google/events each return 401 unauthenticated, while a
  control path under the same prefix returns 404.

NOT verified, stated plainly: the authenticated grant read was not
exercised end-to-end. The named bundle came up signed out ("AUTH_LISTENER:
No saved session") and signing in is not an action I take on someone's
account, so the live grant → events → memories path still needs one manual
run by a signed-in maintainer.

Failure-Class: none
… flow

The new APIClient+GoogleCalendarGrant.swift is the transport that
google-connector-read already exercises through CalendarReaderService, so
it belongs in the same flow's covers list.

Verified: python3 desktop/macos/scripts/check-e2e-flow-coverage.py --strict
--base origin/main -> Covered: 4, Uncovered: 0.
Picks up the three fields the desktop connector needs from
GoogleCalendarEvent (location, description, all_day) and the max_results
ceiling moving 100 -> 500.

Verified: backend/.venv/bin/python scripts/export_openapi.py --surface
app-client --write ../docs/api-reference/app-client-openapi.json -> 19
insertions, 1 deletion, all in GoogleCalendarEvent and its max_results
bound.
Generated artifacts only, required by the openapi-contract gate. Purely
additive: location, description and all_day arrive as optional fields on
GoogleCalendarEvent in each client.

Verified: backend/.venv/bin/python scripts/generate_ts_openapi_types.py ->
4 files, 12 insertions, 0 deletions.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for one concrete, fixable issue — the underlying work is strong, and the diagnosis quality (down to the 120-character sanitizer ceiling and the honest "not verified" section) is exactly what this repo wants in a connector fix.

What I verified as correct

  • backend/routers/google_calendar.py — the new location / description / all_day fields truncate for transport ([:200] / [:300]), and the all-day derivation ('date' in start_raw and 'dateTime' not in start_raw) matches Google's wire shape. All four tests in backend/tests/unit/test_google_calendar_event_response.py pass in CI (confirmed in the suite log).
  • desktop/macos/Desktop/Sources/APIClient+GoogleCalendarGrant.swift — both routes you call exist on main: GET /v1/integrations/{app_key} returns IntegrationResponse {connected, app_key} and GET /v1/integrations/{app_key}/oauth-url returns OAuthUrlResponse {auth_url}, so IntegrationConnectionResponse / IntegrationOAuthURLResponse decode the real wire shapes. The min(max(maxResults, 1), 500) clamp matches the new backend ceiling. GoogleCalendarGrantEventTests.swift decoding into the same CalendarEvent value the cookie reader produced is the right invariant to pin.
  • desktop/macos/Desktop/Sources/CalendarReaderService.swift — grant-first with cookie fallback via try? is the right availability ordering for the #10459 failure modes, and hasGoogleGrant() failing closed to false (never throwing into the read path) is correct.
  • desktop/macos/Desktop/Sources/MainWindow/Components/UserFacingErrorPresentation.swift — replacing the sentence-shape heuristic with raw-error fingerprints (errordomain catching both Error Domain= and bare NS*ErrorDomain) is strictly stronger, and UserFacingErrorPresentationTests.swift driving off the production enums in both directions (curated copy survives, raw system text stays hidden) is exactly how this should be locked down.
  • ConnectorImportOperations.swift extracting saveCalendarEvents and mirroring the connectX consent-then-poll shape reads cleanly; docs/api-reference/app-client-openapi.json matches the backend model changes, the four regenerated omiApi.generated.ts clients are consistent, and the e2e google-connector-read.yaml coverage line plus the changelog entry are appreciated.

The blocking issue: Backend unit suite fails on the desktop REST inventory contract

tests/unit/test_desktop_rest_inventory.py::test_every_in_scope_desktop_rest_route_exists_in_app_client_openapi fails with:

missing: ['/v1/integrations/google_calendar', '/v1/integrations/google_calendar/oauth-url']

That test extracts concrete route literals from every APIClient*.swift source. The new file hardcodes v1/integrations/google_calendar and v1/integrations/google_calendar/oauth-url, but the app-client OpenAPI spec carries these as the templated /v1/integrations/{app_key} and /v1/integrations/{app_key}/oauth-url paths, so the normalized forms can't match. To be clear about severity: this is an SSoT bookkeeping gap, not a runtime one — the live routes are real (your 401 probes and my check of the routers agree). The test's message documents the accepted fixes: interpolate the app key in the Swift route literal so it extracts as the templated path, add the routes to the spec, or list them in KNOWN_MISSING_ROUTES per the convention in that file. Please pick one so the suite goes green.

Non-blocking, while you're in here

  • In connectCalendarViaOAuth, the grant read is readEventsViaGrant(...) ?? [] — if the grant check passes in the poll loop but the inner hasGoogleGrant() re-check misses (transient blip), the import reports success with "Read 0 calendar events". A nil there arguably deserves the "Connected to Google, but reading your calendar failed" branch instead of an empty success.
  • Raising the route-wide max_results ceiling to 500 applies to every client of /v1/calendar/google/events, not just the connector import. It's authenticated and well under Google's 2500 cap, but it's a route-wide API surface change worth a maintainer's explicit nod.

Keeping the cookie reader (retirement tracked on #10459) and not migrating Gmail while gmail.readonly is withheld as a restricted scope were the right scoping calls — I confirmed the withholding comment in integrations_registry.py.

Beyond the CI fix, this reorders desktop Calendar auth to prefer the server-held grant and auto-launches Google's consent screen when both sources fail — a product/auth-direction change that should get maintainer sign-off before merge.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@Git-on-my-level Git-on-my-level added needs-maintainer-review Needs a human maintainer to sign off before merge macOS labels Aug 26, 2026
…key path

test_desktop_rest_inventory extracts route literals from every APIClient*.swift
source and matches them against the app-client OpenAPI spec. Both routes were
written with the app key baked into the literal:

    get("v1/integrations/google_calendar")
    get("v1/integrations/google_calendar/oauth-url")

The spec carries these as /v1/integrations/{app_key} and
/v1/integrations/{app_key}/oauth-url, and _normalize_for_match only rewrites
{...} placeholders -- so a literal key can never match a templated path and the
routes read as missing from the spec:

    E  assert not ['/v1/integrations/google_calendar',
                   '/v1/integrations/google_calendar/oauth-url']

Nothing was actually missing: both templated routes are in the spec and both
backend routes exist (integrations.py:288 and :421). Interpolating the key makes
the Swift say what the contract is -- these are the {app_key} routes with
google_calendar as the key, not routes of their own. The extractor rewrites
Swift interpolation to {param}, which is the same form _normalize_for_match
reduces {app_key} to, so they match.

Chose this over KNOWN_MISSING_ROUTES: nothing is missing, so listing them would
record a gap that does not exist, and the entry would outlive the confusion.

Verified the mechanism rather than just the outcome:

    extracted: /v1/integrations/{param}
               /v1/integrations/{param}/oauth-url
    tests/unit/test_desktop_rest_inventory.py  9 passed (was 1 failed, 8 passed)

Failure-Class: none
@aryanorastar

Copy link
Copy Markdown
Contributor Author

@Git-on-my-level fixed on b8f3c5b5, and thanks for landing on the mechanism rather than just the symptom — "the normalized forms can't match" is the whole bug and it saved me the diagnosis.

I took the first of your three options: interpolate the app key so the literal extracts as the templated path.

private let googleCalendarAppKey = "google_calendar"

get("v1/integrations/\(googleCalendarAppKey)")
get("v1/integrations/\(googleCalendarAppKey)/oauth-url")

_extract_routes_from_swift rewrites Swift interpolation to {param}, which is exactly what _normalize_for_match reduces the spec's {app_key} to, so both sides land on the same string.

I picked it over KNOWN_MISSING_ROUTES because nothing is actually missing — I checked both ends rather than trusting the test's wording. The spec carries /v1/integrations/{app_key} and /v1/integrations/{app_key}/oauth-url, and the backend routes are real (integrations.py:288 and :421). Listing them as known gaps would record something untrue, and that entry would outlive the confusion that produced it. Interpolating also makes the Swift state the contract it's actually using.

Verified the mechanism, not just the green:

extracted: /v1/integrations/{param}
           /v1/integrations/{param}/oauth-url

tests/unit/test_desktop_rest_inventory.py  9 passed   (was 1 failed, 8 passed)

swift build clean, pinned swift-format lint --strict clean, and the full scripts/pre-push gate passes with no skip flags.

On your two non-blocking notes — I've left both, deliberately.

The readEventsViaGrant(...) ?? [] one is a real bug and you described it exactly: a nil from the inner re-check becomes an empty array, so the import reports success with "Read 0 calendar events" when the honest outcome is the failure branch sitting six lines below it. I didn't fix it because I can't reproduce a transient grant miss here, and this repo's rule is to only take an opportunistic fix I can actually verify. It wants either a seam to drive nil through or someone who can force the race. Happy to do it as a follow-up PR with a test rather than an unverified edit inside this one.

The route-wide max_results 500 ceiling is a maintainer call, not mine to quietly keep — flagging it stays flagged.

The auth-direction question you raised at the end (preferring the server-held grant and auto-launching consent when both sources fail) is also still open and still a human's.

@aryanorastar

Copy link
Copy Markdown
Contributor Author

Follow-up on the two red checks now showing here — Hygiene and Build. Neither is this PR.

error: lockfile had changes, but lockfile is frozen
Manifest checks failed: web-app-checks

64db30c791 added prettier and prettier-plugin-tailwindcss to web/app/package.json without regenerating web/app/bun.lock. web/app/test.sh installs frozen, so it fails. Reproduces on a pristine checkout of main with no branch involved:

mkdir -p /tmp/mainweb
git archive origin/main web/app | tar -x -C /tmp/mainweb
cd /tmp/mainweb/web/app && bun install --frozen-lockfile

This PR only surfaces it: web-app-checks triggers on web/app/src/**, and the one web file here is a regenerated omiApi.generated.ts. The other open PRs are green because none of them trip those trigger paths — the break is latent on main, not specific to anything here.

Fix is up as #12288 — six lines, the two declared deps plus their resolution entries, no transitive drift. bun install --frozen-lockfile succeeds against a cleared node_modules and web/app/test.sh passes 66 files / 394 tests on it. This PR goes green once that lands.

I also merged current main in (8449c63e) while diagnosing this, since the branch was 126 commits behind. Diff scope is unchanged — still exactly the 15 files this PR owns, no ride-alongs.

The route-inventory fix from b8f3c5b5 is unaffected and still stands: test_desktop_rest_inventory was the one genuine failure and it now passes 9/9.

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real bug fix (Calendar OAuth grant path + truncated error message, issue #10459) from a trusted backend contributor, but mixed backend+desktop+web diff is 514 lines (>300 ceiling) and Backend unit suite is currently failing (pre-existing BYOK test breakage on main, unrelated to this diff) — confidence gate not met at 4/5, approve only per policy, not merging.

@aryanorastar

Copy link
Copy Markdown
Contributor Author

Still in progress, but the implementation itself is complete.

The requested route-inventory change remains fixed in b8f3c5b5 (the focused inventory suite went from 1 failure / 8 passes to 9/9), and the current PR head has every reported check green except Backend unit suite.

That remaining red is inherited from main: backend/tests/unit/test_byok_security.py fails 8 tests there because its mocks and partial-enrollment assertions predate the current per-provider BYOK contract. I have repaired that upstream, separately and test-only, in #12302:

main:       8 failed, 96 passed
#12302:     104 passed
preflight:  25/25 selected checks passed

Once #12302 lands and this PR's backend job is rerun/rebased, the only remaining gates are maintainer product-direction sign-off and the stale requested-changes review state.

@Git-on-my-level need human response: your concrete blocking request was addressed in b8f3c5b5; please clear or update the old review state when you can.

@Git-on-my-level
Git-on-my-level dismissed their stale review August 27, 2026 22:09

Resolved on the current head (64088db): the hardcoded v1/integrations/google_calendar literals are now interpolated through googleCalendarAppKey, so the desktop REST inventory extraction normalizes to the templated /v1/integrations/{app_key} paths — test_every_in_scope_desktop_rest_route_exists_in_app_client_openapi passes in this head's Backend unit suite log. This review was the only remaining requested-changes state, so it is being dismissed as stale.

@Git-on-my-level

Copy link
Copy Markdown
Collaborator

Follow-up on the requested changes from my earlier review — re-verified on this head (64088db) before clearing it.

Route-inventory blocker: resolved and verified. Interpolating googleCalendarAppKey into both route literals in APIClient+GoogleCalendarGrant.swift was the right pick of the three options — nothing was actually missing from the spec, so a KNOWN_MISSING_ROUTES entry would have recorded something untrue. Confirmed against this head's Backend unit suite log: the desktop REST inventory suite passes, including the previously failing test_every_in_scope_desktop_rest_route_exists_in_app_client_openapi, and all four test_google_calendar_event_response.py tests pass.

The remaining Backend unit suite red is not this PR. The 8 failures are all in tests/unit/test_byok_security.py (utils.subscription has no attribute get_byok_keys, plus enrolled-provider fingerprint assertions) — a file this PR never touches, with mocks that predate the current per-provider BYOK contract. Your diagnosis matches what I see: main's own backend job was red earlier today and has been green since 19:41Z, so the repair is already on main — a rebase (or re-run once it reaches the merge ref) should take this PR green with no further changes here. #12302 may now be redundant with what landed on main; worth a quick check before spending more time on it.

With the maintainer approval on this head covering the auth-direction change (grant-first with cookie fallback, the consent-screen launch when both sources are exhausted, and the route-wide max_results ceiling raise to 500), I have no remaining automated blockers — the only gate left is a green CI run after the rebase. Clearing the stale requested-changes state and adding positive-signal; needs-maintainer-review comes off now that the sign-off has landed. Thanks for the follow-through on this one — the status comments root-causing two unrelated main-side CI breaks made re-review fast.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@Git-on-my-level Git-on-my-level added positive-signal Good PR — positive signal, not a formal approval and removed needs-maintainer-review Needs a human maintainer to sign off before merge labels Aug 27, 2026
@aryanorastar

aryanorastar commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed on current main — requested fix and proof rerun

This branch is now conflict-free on top of current main (cdca0c865f). The diff remains the same 15 owned files; no iOS or Android code was added. GitHub reports the new head MERGEABLE and the existing approval remains in place.

The requested route-inventory correction is still present: the Swift client expresses the Google Calendar app key through interpolation, so static extraction normalizes the concrete calls to the OpenAPI templates rather than reporting two false missing routes.

Automated proof

  • macOS UserFacingErrorPresentationTests + GoogleCalendarGrantEventTests: 9 passed, 0 failed
  • backend Calendar event response tests: 4 passed, 0 failed
  • desktop REST/OpenAPI inventory tests: 9 passed, 0 failed
  • web app lane: 72 files passed, 415 tests passed
  • pinned Swift format: changed Swift files clean; full repository scope clean
  • pinned SwiftLint: 0 violations in 1,427 files
  • Black 26.5.1: 2 changed Python files unchanged
  • desktop e2e source coverage: 4/4 changed Swift files covered
  • git diff --check origin/main...HEAD: passed
  • make preflight: 36/36 checks passed

Local-runtime note: this machine uses Node 26, whose native localStorage global is undefined unless a backing file is supplied. The first preflight run therefore failed nine untouched web tests in beforeEach(localStorage.clear()); rerunning with a local-storage file made the complete lane pass 415/415. No web test or runner code is changed by this PR.

Remaining human verification

The automated auth boundary is covered, but the signed-in grant path still needs one real account run:

  1. On macOS, start signed in to Omi with Google Calendar not connected.
  2. Open Apps and choose Calendar connect.
  3. Complete Google consent in the browser.
  4. Return to Omi and verify the connector reaches connected/import success.
  5. Confirm imported events retain title, start/end, location, description, and all-day state.
  6. Capture the before/after connector state for PR evidence.

Fresh CI is running on head c659fd789e. Once CI and that authenticated smoke are green, the only remaining decision is maintainer sign-off on grant-first Calendar auth and the route-wide max_results=500 ceiling. @Git-on-my-level @undivisible

@Git-on-my-level

Copy link
Copy Markdown
Collaborator

Reviewed across the backend router, the macOS connector path, and the generated clients. This fixes the #10459 failure modes the right way: the desktop stops depending on decrypting Google cookies out of a Chromium profile and reads through the server-held Calendar grant instead, with the cookie reader kept as a fallback so nobody working today regresses. Grant-first ordering in CalendarReaderService.readEvents, the fallback on both absent-grant and failed grant read, and the backend-mediated consent flow in ConnectorImportOperations.connectCalendarViaOAuth all look sound, and the wire shape is tested on both sides.

Notes, none blocking:

  • backend/routers/google_calendar.py — the location/description truncation and all_day detection are correct and now unit-tested in backend/tests/unit/test_google_calendar_event_response.py; raising the max_results ceiling to 500 is documented in the OpenAPI spec and matches the desktop's year-back import. Token refresh and per-user auth on this route are untouched.
  • desktop/macos/Desktop/Sources/APIClient+GoogleCalendarGrant.swift — clean transport layer; clamping maxResults to the endpoint's 500 ceiling and percent-encoding the time bounds are the right details, and GoogleCalendarGrantEventTests.swift pins the decode into CalendarEvent so a silent field drop can't ship.
  • desktop/macos/Desktop/Sources/MainWindow/Components/UserFacingErrorPresentation.swift — switching the sanitizer from sentence-shape heuristics (trailing period, <120 chars, prefix list) to raw-error fingerprints (errordomain, ://, status codes, colon counts) is the right call; testEveryGoogleConnectorErrorSurvivesDisplaySanitization driving off the production error enums and testStillHidesRawSystemErrorText pinning the leak direction are exactly how to keep it honest. Two edges to keep in mind for future copy: any curated string containing the substring "errordomain" will now be rejected, and copy in the 120–160 char band without the old prefixes is newly preserved.
  • desktop/macos/Desktop/Sources/MainWindow/Pages/ConnectorImportOperations.swift — in connectCalendarViaOAuth, the 60×2s poll goes through hasGoogleGrant(), whose backend failure folds to "no grant", so a prolonged backend outage during the poll always ends in "Didn't hear back from Google" rather than surfacing the underlying connectivity error. Fine for a fallback path; just noting the diagnosis is lossy there. Auto-launching consent only after both auth sources are exhausted is a good touch.
  • CalendarReaderService.verifyConnection now short-circuits .connected on a grant-backed 1-event fetch before the cookie probe — deliberate status-behavior change, consistent with the grant-first read; worth remembering when triaging future "Calendar shows connected" reports.
  • The remaining files (the changelog entry, the e2e flow cover in google-connector-read.yaml, the four omiApi.generated.ts mirrors, and docs/api-reference/app-client-openapi.json) all check out; the generated diffs match the pydantic model exactly.

Thanks — a well-scoped fix with real tests on both sides of the wire.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macOS positive-signal Good PR — positive signal, not a formal approval

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants