feat(calendar): Phase 2 M6 — multi-account Google + sync health UI - #289
Merged
Conversation
Multi-account groundwork for Google Calendar. The keytar service is unchanged but each token slot is now keyed off `<kind>-<accountId>[-<MEMRY_DEVICE>]` so two Google accounts on the same machine can coexist without overwriting each other. `storeGoogleCalendarTokens` / `getGoogleCalendarTokens` / `hasGoogleCalendarTokens` / `clearGoogleCalendarTokens` now take an accountId. A transitional `LEGACY_DEFAULT_ACCOUNT_ID` placeholder keeps existing single-account callers compiling and passing tests; M6 T2 will replace every reference with the real Google profile email returned by userinfo. Tests: keychain.test.ts asserts that two accounts land in distinct slots, that clearing one leaves the other intact, and that MEMRY_DEVICE suffixing layers cleanly on top of accountId partitioning. Closes part of G9 (single-Google-account-only).
… T2)
Adds the userinfo fetch (`oauth2/v2/userinfo`) right after token
exchange and uses the returned `email` as the canonical accountId for
both keychain partitioning and the `calendar_sources.kind='account'`
row. Connecting a second Google account on the same device now creates
a second source row instead of overwriting the first; tokens land in
distinct keychain slots; either account can be disconnected on its own.
Multi-account plumbing:
- `connectGoogleCalendar()` returns `{ accountId, account: { email, … } }`.
- `disconnectGoogleCalendar(accountId)` is now per-account; revokes and
clears keychain only for that account. Calendar handler enumerates
every connected Google account and disconnects each in turn.
- `createGoogleCalendarClient({ accountId })` requires accountId at
construction; per-account `pendingRefreshes` map prevents
cross-account token refresh races.
- New `resolveDefaultGoogleAccountId(db)` / `listGoogleAccountIds(db)`
/ `hasAnyGoogleCalendarLocalAuth(db)` helpers in `oauth.ts` for
callers that need to address "any connected account" until M6 T4
threads per-target routing through push and sync.
- `session-teardown.ts` enumerates accounts on logout instead of
hitting one fixed slot.
- `LIST_GOOGLE_CALENDARS` IPC returns an empty list when no Google
account is connected (instead of throwing on the missing default).
Tests:
- Existing successful-connect scenario asserts the userinfo fetch and
email-keyed keychain.
- New scenario: `connecting a second Google account stores tokens
under a distinct accountId without overwriting the first` —
disconnects account A and confirms account B's tokens survive.
- `calendar-handlers.test.ts` mocks the new oauth exports and the
connect mock returns the M6 shape.
Closes most of G9; the remaining pieces (sync-health surface, per-
calendar push routing, source-row tombstone scoping) land in T3–T6.
Whitespace-only collapsing of multi-line array literals and JSX prop spreads in M5 calendar files. No functional changes; formatter re-applies them every save so landing them clears the working tree ahead of M6 T3+.
Adds `CalendarProviderAccountStatus` to the calendar contract and an
`accounts: CalendarProviderAccountStatus[]` field on
`CalendarProviderStatus`. Each entry surfaces:
{ accountId, email, status, lastSyncedAt, lastError }
`status` collapses the underlying `calendar_sources.syncStatus` enum
into the renderer-friendly `'connected' | 'disconnected' | 'error'`
projection: any account whose keychain still has tokens and whose
source row isn't in `'error'` reads as `'connected'`; rows in `'error'`
expose the latest message via `lastError`; rows whose tokens have been
revoked at the OS keychain (e.g. user-revoked from Google) downgrade
to `'disconnected'` even if the DB still has the source row. `email`
falls back to `source.title` when the metadata column is missing.
The legacy single `account` field stays on the contract so the
existing renderer keeps working until the rest of M6 lands; new code
should prefer `accounts`.
Renderer:
- `GoogleCalendarIntegrationRow` renders a chip per account using the
array — colour-coded per status, last-error text truncated at 60
chars, full message in the chip's `title` tooltip. Each chip has
`data-testid` + `data-account-status` so tests + future health
surfaces can target them deterministically.
Tests:
- `calendar-handlers.test.ts`: new "returns one account in
status.accounts per connected Google account" inserts two account
rows (one with `syncStatus='ok'`, one with `syncStatus='error'`)
and stubs per-account keychain lookups to confirm the
`connected | disconnected | error` projection. Existing fixtures
pick up `accounts: []` / `accounts: [...]`.
- `google-calendar-integration-row.test.tsx`: new "renders one chip
per connected Google account" asserts both emails render and chip
attributes match (status='connected' for Alice, status='error'
with lastError text for Bob).
Closes G10's IPC half; renderer health surface (sync-health widget,
retry button) lands in M6 T6.
Pushes that previously always used "the one default account's" client
now resolve the right account from the target calendar's source row.
The new helper `resolveTargetGoogleAccountId(db, target,
existingBinding)` walks:
1. existingBinding.remoteCalendarId → calendar source's accountId
2. for events: calendarEvents.targetCalendarId → calendar source's
accountId
3. fallback: resolveDefaultGoogleAccountId(db)
`pushSourceToGoogleCalendar` and `deleteSourceFromGoogleCalendar` now
ask the helper before constructing the client. `getGoogleClient`
takes an optional `accountIdOverride` so the routing decision flows
all the way to keychain — events bound to account A's calendar push
through account A's tokens; events bound to account B's calendar
push through account B's. Tests with `deps.client` short-circuit the
factory and stay routing-agnostic, so existing M2/M3/M5 sync-service
test scenarios keep working unchanged after extending their oauth
mock.
Tests:
- New `account-routing.test.ts` covers all four resolution branches:
binding-priority, event.targetCalendarId, default fallback, and the
explicit "no account anywhere" null case.
- `sync-service.test.ts` oauth mock now exposes
`resolveDefaultGoogleAccountId`, `hasAnyGoogleCalendarLocalAuth`,
and `listGoogleAccountIds` so the routing helper has an answer in
unit-test fixtures.
Closes the routing half of G9. Per-account scoped disconnect
(M6 T5), sync-health UI (M6 T6) still pending.
Adds an optional `accountId` field to `CalendarProviderRequestSchema`.
When the renderer (M6 T6 will use this from the per-account chip's
overflow menu) supplies it, only that account is disconnected:
- the per-account keychain entries (revoke + clear) via the now
per-account `disconnectGoogleCalendar(accountId)`
- only that account's `kind='calendar'` rows + the `kind='account'`
parent are tombstoned (set `archivedAt`); we use update-with-
`syncCalendarSourceUpdate` instead of delete so the row's clock +
E2EE sync envelope flow the archive to other devices
- only push channels whose parent calendar source belongs to the
removed account are stopped via
`pushRuntime.handleSelectionToggle({…, isSelected: false})`. Other
accounts' channels stay live.
- bindings + external-event mirror rows scoped by `remoteCalendarId`
to the removed account's calendars are removed (hard-delete is
fine here — the deleted-row tombstone already syncs)
- the global sync runner is **not** stopped (other accounts still
need it)
Without `accountId`, the legacy nuke-all path keeps its prior
semantics so the existing big "Disconnect" button remains a
no-confirmation provider-wide reset.
Tests:
- new "disconnects only the requested accountId, leaving other
accounts intact" seeds Alice+Bob accounts each with one calendar,
fires `DISCONNECT_PROVIDER` with `accountId: alice@example.com`,
asserts (a) `disconnectGoogleCalendar` was called exactly once
with Alice's id, (b) Alice's source rows disappear from
`LIST_SOURCES` (filtered by `archivedAt`), and (c) Bob's account +
calendar rows are still live.
Closes G10. Adds the per-source health surface, persists the last
sync failure to a real column, and exposes a "Retry now" IPC so the
renderer can re-kick a single source.
Schema (migration 0028):
- `calendar_sources.last_error TEXT` — nullable. Hand-written SQL +
journal entry per the project convention since `pnpm db:generate`
drifts on these tables.
- `CalendarSourceRecord` gains `lastError: string | null`.
Sync-service:
- `syncGoogleCalendarSource` now wraps in `recordSyncError` on any
uncaught failure: writes `syncStatus='error'` + truncated 200-char
message to `lastError`, fires `markSyncedTableMutation` so the
failure propagates cross-device.
- On a clean run, `syncStatus='ok'` + `lastError=null` clears any
prior failure.
IPC:
- New `CalendarChannels.invoke.RETRY_GOOGLE_CALENDAR_SOURCE_SYNC`
('calendar:retry-google-source-sync') + `RetryCalendarSourceSync`
schema + response shape (`{ success, source, error? }`).
- Handler validates the source is `provider='google'`, kind='calendar',
invokes `syncGoogleCalendarSource(db, sourceId)`, returns the
refreshed `CalendarSourceRecord` so the renderer can update its
cache without a separate refetch.
Renderer:
- `GoogleCalendarSourcePicker` upgraded into the per-source health
view: status dot (ok=emerald / pending=amber / error=destructive /
idle=muted), status label, and — when status='error' — a "Retry
now" button + truncated error message (full text in the row's
`title` tooltip).
- `GoogleCalendarIntegrationRow` wires `retryGoogleCalendarSourceSync`
into a `useMutation` and forwards `onRetrySource` /
`retryingSourceId` props to the picker.
- `buildProviderAccountStatus` now reads `lastError` from the
dedicated column first, falling back to the M5-era `metadata.lastError`
for any legacy rows that still carry it there.
Tests:
- sync-service: "writes truncated lastError + syncStatus='error' when
listEvents throws" (asserts the 200-char cap), and "clears
lastError on a successful sync after a previous error".
- calendar-handlers: "RETRY_GOOGLE_CALENDAR_SOURCE_SYNC fires
syncGoogleCalendarSource and returns the refreshed source".
- google-calendar-integration-row: "shows a Retry button + lastError
on calendar sources in error state and fires retry IPC".
Generated IPC bindings re-run via `pnpm ipc:generate` to pick up the
new RETRY channel; `pnpm ipc:check` is green.
Out of scope here: dedicated `calendar_sync_status_changed`
projection event (current `CHANGED` event already triggers renderer
refetch via TanStack invalidation) and the calendar-header status
chip — both follow-ups.
… T7) `sync-service.ts` crossed the 800-line eslint cap after M6 T6's sync-health additions. Extracted three pure functions to a new sibling file `push-conflict-retry.ts`: - `loadSourceAsGoogleEvent(db, target)` — maps a CalendarSyncTarget to a `GoogleCalendarUpsertEventInput` - `pushEventWithConflictRetry(db, target, client, calendarId, binding)` — the M3 412 conflict loop (3 retries, merges remote, marks binding `remoteVersion='conflict'` on exhaustion) - `mergeRemoteEventIntoLocal(db, eventId, remote)` — the M3 field- clock-aware merge that keeps the local doc clock untouched No behaviour change. sync-service drops from 949→760 lines; the extracted module is ~180 lines and self-contained (no circular imports — it depends on mappers, field-merge, and change-events but not on sync-service). Pre-existing test surface continues to cover both files.
Collaborator
Author
Code reviewFound 4 issues:
memry/apps/desktop/src/main/calendar/google/sync-service.ts Lines 756 to 772 in b607d07
memry/apps/desktop/src/main/calendar/google/sync-service.ts Lines 134 to 145 in b607d07 memry/apps/desktop/src/main/calendar/google/sync-service.ts Lines 209 to 220 in b607d07
memry/apps/desktop/src/main/calendar/google/push-runtime.ts Lines 86 to 97 in b607d07
🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
M6 of the Google Calendar Phase 2 plan. Closes the last two open gaps from the audit:
Each commit follows TDD (RED → GREEN) and is independently green; final architecture refactor extracted push-conflict-retry to keep
sync-service.tsunder the 800-line eslint cap.Commit-by-commit
41ce2840partition keytar entries by accountIdb33960b8OAuth uses profile email as accountId1a4fef03provider status IPC returns per-account array8a3a6935route push by target calendar's account56454c07scope DISCONNECT to a single account6db947d8sync-health surface + lastError column + retry IPCb607d079extract push-conflict-retry from sync-service(Plus
8150e4f4chore: absorb prettier reformats from prior session)Architecture highlights
<kind>-<accountId>[-<MEMRY_DEVICE>]so two Google accounts coexist without overwriting tokens. The newLEGACY_DEFAULT_ACCOUNT_IDconstant landed in T1 as a transitional shim and was removed in T2 onceuserinfobecame the canonical source ofaccountId.connectGoogleCalendar()callsoauth2/v2/userinfoimmediately after token exchange. The returnedemailbecomes both the keychain partition and thecalendar_sources.accountId. Connecting a second account creates a second source row instead of overwriting the first.resolveTargetGoogleAccountId(db, target, existingBinding)walks (binding → event.targetCalendarId → default account) to determine which account's client to construct for each push.getGoogleClientaccepts an optional accountId override so the routing decision flows all the way to keychain.DISCONNECT_PROVIDERaccepts an optionalaccountId. With it set, only that account's keychain is wiped, only its source rows are tombstoned (archivedAt), and only its push channels are torn down. Other accounts stay live. Without it, the legacy nuke-all path is preserved for the existing big "Disconnect" button.syncStatus='error'+ a 200-char-truncatedlastErrorto a new column (migration 0028). The renderer shows a status dot per calendar source and a "Retry now" button when status is error; clicking it invokessyncGoogleCalendarSource(id)via the newRETRY_GOOGLE_CALENDAR_SOURCE_SYNCIPC.accounts: CalendarProviderAccountStatus[]so the renderer can render one chip per connected account with email + status + lastError.Test plan
pnpm --filter @memry/desktop exec vitest run src/main/calendar src/main/sync src/main/ipc src/renderer/src/components/calendar src/renderer/src/components/settings— 1149/1149 passing (108 files)pnpm --filter @memry/desktop typecheck:node && pnpm --filter @memry/desktop typecheck:web— both cleanpnpm lint— 0 errors (912 pre-existing warnings carry over)pnpm ipc:check— generated bindings up-to-datecalendar_sources.last_error TEXT) verified by 1149 vitest runs throughcreateTestDataDb's in-memorymigrate()pass — every test that INSERTs intocalendar_sourcesexercises the migrationOut of scope (follow-ups)
calendar_sync_status_changedprojection event — currentCHANGEDevent withentityType: 'calendar_source'already triggers TanStack invalidation, so the renderer refetches on every status change.accountIdarg), but UI affordance is a follow-up; current behavior keeps the big provider-wide "Disconnect" button.Known gotchas
pnpm db:generateis not used (project gotcha, hand-write migrations since 0020). 0028 is hand-written + journal entry added manually.sync-service.tspost-T6 crossed the 800-line eslint cap; T7 extractspush-conflict-retry.tsto bring it back under (760 lines).