feat: classify auth for google reconnect on server#1535
Conversation
|
@cursoragent Please pick up where I left off by implementing the plan in the issue body and using your best judgement until it's at a satisfactory level |
Implementation summaryCore changes
Testing
Documentation
StatusThe implementation follows the plan:
The code is ready for review and testing. All tests pass, and the changes have been committed and pushed to the |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Four unused imports left after removing determineAuthMethod
- Removed the four unused imports (error, getSync, canDoIncrementalSync, findCompassUserBy) that were left behind after determineAuthMethod was removed.
…mination - Renamed `reconnectGoogleForSession` to `repairGoogleConnection` to better reflect its purpose in handling Google connection repairs. - Enhanced the logic for determining the auth mode based on server-side state, replacing reliance on frontend intent. - Updated documentation to clarify the new auth mode classification and deprecated fields. - Refactored related tests to align with the new method names and logic, ensuring comprehensive coverage of the updated functionality.
- Updated various interface definitions across the codebase to improve formatting and readability by aligning type extensions and removing unnecessary line breaks. - Adjusted interfaces in event, user, and component types to follow a consistent style, enhancing maintainability and clarity.
- Add explicit handling for undefined/empty events arrays - Add comprehensive JSDoc documentation - Ensure function returns false when sync data is missing or incomplete - This ensures reconnect_repair path is correctly triggered when sync is unhealthy Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>
Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>
Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>
Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>
…functions - Eliminated the sessionUserId field from GoogleSignInSuccess type and related functions, as auth mode is now determined server-side. - Removed the resolveGoogleSessionUserId function and its associated tests, streamlining the Google authentication process. - Updated the useGoogleAuth hook to reflect these changes, ensuring a cleaner implementation without reliance on frontend-provided intent. Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>
…ming - Added a guideline for using uppercase and underscores for constant names, exemplified by `SIGNIN_INCREMENTAL`. - Updated documentation to reflect the new naming conventions for auth modes: `SIGNUP`, `SIGNIN_INCREMENTAL`, and `RECONNECT_REPAIR`. - Ensured consistency in the codebase by renaming references to auth modes in tests and service files. Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>
073bfda to
9030f1d
Compare
- Updated the Google authentication process to eliminate reliance on the `googleAuthIntent` from the frontend, making the server the authoritative source for auth mode determination. - Refactored related components and hooks to reflect this change, ensuring a cleaner and more consistent implementation. - Adjusted tests to align with the new logic, removing references to the deprecated intent and enhancing overall test coverage. Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Repair button stays enabled allowing duplicate API calls
- Fixed by including isImportPending in the isImporting calculation, which immediately disables the repair button when clicked, preventing duplicate API calls during the gap before the websocket event arrives.
- Streamlined interface definitions across various files to enhance readability by aligning type extensions and removing unnecessary line breaks. - Updated interfaces in event, user, and component types to follow a consistent style, improving maintainability and clarity.
6e1cf62 to
4b24db2
Compare
- Added a test to ensure that an import request is not automatically triggered when the Google connection status is "reconnect_required". - Updated the logic in the useGcalSync hook to only auto-request an import when the connection status is "connected". - Improved test coverage for various connection scenarios, ensuring robust handling of metadata updates.
- Updated all instances of sync status values in the codebase to use uppercase naming conventions: "IMPORTING", "ERRORED", "COMPLETED", and "RESTART". - Adjusted related tests and documentation to reflect these changes, ensuring consistency across the application. - Improved clarity and maintainability of the sync logic by adhering to a unified naming standard.
…form submission - Updated Playwright configuration to utilize 2 workers for parallel test execution, enhancing performance. - Modified event form submission logic to use `locator.press()` for sending key events, ensuring focus remains on the input field during CI tests. - This change addresses potential issues with form submission visibility and reliability in automated testing environments.
| compassUserId: string | null; | ||
| hasStoredRefreshToken: boolean; | ||
| hasHealthySync: boolean; | ||
| createdNewRecipeUser: boolean; |
There was a problem hiding this comment.
Unused fields in AuthDecision type add dead code
Low Severity
The AuthDecision type includes hasStoredRefreshToken, hasHealthySync, and createdNewRecipeUser fields that are computed inside determineAuthMode but never read by handleGoogleAuth. The consumer only accesses decision.authMode and decision.compassUserId, making the other three fields dead code that increases cognitive overhead without adding value.
Additional Locations (1)
91e3d81 to
90d762d
Compare




Implements the server-owned Google auth finalization cleanup.
SIGNUP/SIGNIN_INCREMENTAL/RECONNECT_REPAIR)Follow-Up Plan
This PR follows the server-owned auth finalization plan: move Google auth finalization fully server-side so the backend owns
SIGNUP/SIGNIN_INCREMENTAL/RECONNECT_REPAIRselection, reconnect repair, metadata updates, and sync kickoff; keep the client limited to launching OAuth, submitting the payload, and uploading device-local events after success.Note
High Risk
Changes Google OAuth finalization and sync repair decisions on the backend and updates status enums across core/backend/web; mistakes could misroute auth flows or leave users stuck in repair/import states. Broad cross-package status casing changes increase integration/regression risk despite added tests and backward-compatible normalization.
Overview
Moves Google OAuth flow routing to be server-owned:
handleGoogleAuthnow classifiesSIGNUPvsSIGNIN_INCREMENTALvsRECONNECT_REPAIRusing persisted user state (refresh token + sync health) and invokesgoogleSignup,googleSignin, or newrepairGoogleConnectionaccordingly; client-provided reconnect intent is removed from both web payloads and backend SuperTokens middleware.Standardizes Google sync/import statuses to uppercase across shared types (
UserMetadatasync flags,GoogleSyncStatus, and websocketImportGCalEndPayload) and addsnormalizeSyncStatus()to tolerate legacy lowercase values while evaluating import/repair logic.Updates web UI/auth and socket sync behavior to reflect server metadata/events (no local reconnect intent; avoid treating
isImportPendingas importing; don’t auto-request import whenreconnect_required), adjusts backend sync/import paths to emit the new status values, and fixes Playwright flakiness by pressing the save shortcut via the focused title input; documentation and tests are updated to match.Written by Cursor Bugbot for commit 91e3d81. This will update automatically on new commits. Configure here.