Conversation
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
There was a problem hiding this comment.
Builder reviewed your changes and found 4 potential issues 🟡
Review Details
Code Review Summary
PR #5569 changes Calendar event identity and mutation routing so duplicate Google events prefer writable sources, while opaque account-scoped IDs preserve the connection needed for reads and mutations. The ranking and explicit account-mismatch checks are a sound direction, and the added regression coverage exercises the central duplicate and bulk-account cases. This is a standard-risk business-logic change because it affects provider reads and event mutations.
Findings
- 🟡 Four medium-severity issues need attention:
- Account-scoped get-event failures are converted into a misleading not-found result instead of preserving auth, rate-limit, transient, or provider-error provenance.
- Explicitly selected primary-calendar paths can still emit legacy IDs when multiple accounts are queried, allowing same-provider-ID collisions.
- Mutation result IDs can lose the account binding after an opaque event is updated/deleted/RSVP'd, making follow-up actions ambiguous.
- Bulk mutations can mix opaque account-scoped IDs with legacy unscoped IDs and silently apply the inferred account to the legacy IDs.
The core source-ranking approach is deterministic and correctly prioritizes writable access over arrival order. No schema, credential, or migration changes were introduced.
🧪 Browser testing: Could not verify — the dev server was healthy, but browser automation tools were unavailable and no safe multi-account fixture existed; all planned cases were reported as environment-blocked.
| const selectedClients = accountEvent | ||
| ? clients.filter( | ||
| ({ email: accountEmail }) => | ||
| accountEmail.trim().toLowerCase() === accountEvent.accountEmail, | ||
| ) | ||
| : clients; | ||
| for (const { email: acctEmail, accessToken } of selectedClients) { |
There was a problem hiding this comment.
🟡 Preserve provider failures for account-scoped event lookups
When an opaque account-scoped ID restricts selectedClients to the encoded account, the existing catch still swallows every calendarGetEvent failure and the action eventually reports Event not found. A revoked token, 403, 429, or transient provider outage is not a confirmed 404; preserve the underlying error or return typed provider-error provenance for this account-scoped path.
Additional Info
Found by 1 of 3 code-review agents; confirmed against the account-restricted lookup and catch flow.
| calendarSource && !calendarSource.primary | ||
| ? `google-${calendarSource.sourceKey}-${event.id}` | ||
| : `google-${event.id}`, | ||
| : !calendarSource && clients.length > 1 |
There was a problem hiding this comment.
🟡 Scope selected primary events by account
When calendarSourceKeys are supplied, primary source paths still take the legacy google-${event.id} branch because the account-scoped fallback only runs when !calendarSource. If two connected primary accounts expose the same provider event ID, the rows collide and get-event has no account binding to distinguish them. Use an account-scoped ID whenever multiple accounts are queried, including explicitly selected primary paths, while retaining legacy IDs only for the single-account compatibility case.
Additional Info
Found by 1 of 3 code-review agents; confirmed from the ID branch and multi-account primary-source behavior.
| const accountEmail = await resolveOwnedAccountEmail( | ||
| args.accountEmail, | ||
| resolveGoogleEventAccountEmail(args.id, args.accountEmail), | ||
| ownerEmail, | ||
| ); | ||
| const googleEventId = normalizeWritableGoogleEventId(args.id); |
There was a problem hiding this comment.
🟡 Preserve account-scoped IDs in mutation results
An opaque event ID is resolved to its account here, but the existing success/replacement paths later return google-${googleEventId}, dropping that binding. A follow-up update, delete, or RSVP using the returned ID can resolve the same provider ID against another connected account. Preserve the original opaque ID or reconstruct it with the bound account, and apply the same rule to the analogous delete, RSVP, and bulk mutation result paths.
Additional Info
Found by 1 of 3 code-review agents; the issue applies to the shared mutation-result contract across the affected actions.
| export function resolveBulkGoogleEventAccountEmail( | ||
| ids: string[], | ||
| accountEmail: string | undefined, | ||
| ): string | undefined { | ||
| const accounts = new Set( | ||
| ids | ||
| .map((id) => resolveGoogleEventAccountEmail(id, accountEmail)) | ||
| .filter((email): email is string => !!email) | ||
| .map((email) => email.trim().toLowerCase()), | ||
| ); | ||
| if (accounts.size > 1) { | ||
| throw new Error("Bulk event ids must belong to one Google account"); | ||
| } | ||
| return accounts.values().next().value ?? accountEmail; |
There was a problem hiding this comment.
🟡 Reject mixed scoped and legacy IDs in bulk mutations
This helper infers one account from any opaque ID and returns it for the whole batch, even when other IDs are legacy google-<provider-id> values with no account provenance. With multiple accounts exposing the same provider ID, those legacy IDs can refer to another account but will be fetched and mutated on the inferred account. Reject mixed scoped/unscoped batches or require an explicit account for every unscoped ID.
Additional Info
Found by 1 of 3 code-review agents; confirmed from the set construction and fallback return behavior.
Visual recap — generation failedThe visual recap could not be generated for this pull request. This is informational only and does not block the PR. Diagnostic: No plan URL: Repair changed too much of targeted file plan.mdx; expected a localized parser fix. Agent output: Repaired |
This branch has not been deployed
1 participant
Problem
When the same Google calendar is visible through both an owner account and a reader account, Calendar could keep whichever duplicate event arrived first. If the reader copy won, an event the user owns appeared read-only and its edit and delete actions disappeared.
Approach
Choose the strongest source for a duplicated provider event: writable access first, then primary ownership, access role, and a deterministic account fallback. Preserve account identity in multi-account event IDs so subsequent reads and mutations return to the connection that supplied the winning event.
What changed
Safety and operations
The opaque identity is limited to ambiguous multi-account results; existing single-account Google event IDs remain compatible. There are no schema changes, migrations, credential changes, or deployment steps.
Verification
vitest --runacross the nine affected Calendar suites: 190 tests passedguard:no-silent-coercionpassedguard:external-result-contractpassedA live personal Google event was not destructively edited or deleted; provider routing and mutation rejection are covered at the action boundary.
Review focus