-
-
Notifications
You must be signed in to change notification settings - Fork 19
Set up Background sync using Trigger.dev #3092
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update introduces a coordinated system for managing calendar synchronization within workspaces, including a new database table, sync control logic, and integration with both background and user-initiated sync flows. It adds new scheduled and trigger tasks, updates type declarations, and provides supporting configuration and documentation files. Environment variable handling and package dependencies are also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant API
participant TriggerTask
participant Supabase
participant GoogleCalendar
User->>WebApp: Click "Trigger Hello World"
WebApp->>API: GET /api/hello-world
API->>TriggerTask: tasks.trigger('hello-world', 'James')
TriggerTask->>TriggerTask: Run helloWorldTask
TriggerTask-->>API: Result (Hello, world!)
API-->>WebApp: JSON response
WebApp-->>User: Log result
User->>WebApp: Initiate Calendar Sync
WebApp->>Supabase: canProceedWithSync(wsId)
alt Cooldown active
Supabase-->>WebApp: Block sync (cooldown)
WebApp-->>User: Show cooldown message
else Cooldown expired
Supabase-->>WebApp: Allow sync
WebApp->>GoogleCalendar: Fetch events
GoogleCalendar-->>WebApp: Events data
WebApp->>Supabase: Upsert events
WebApp->>Supabase: updateLastUpsert(wsId)
WebApp-->>User: Sync complete
end
Note over TriggerTask, Supabase: Background sync uses same coordination logic to avoid conflicts
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/types/src/supabase.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs ✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🧹 Nitpick comments (12)
package.json (1)
57-58
: Consider exposing a convenience script for Trigger.dev local devYou added
@trigger.dev/sdk
but developers still have to remember
npx trigger.dev@latest dev
. A small script alias (e.g."trigger:dev": "trigger.dev dev"
) improves DX and keeps the command in sync with the pinned version."scripts": { + "trigger:dev": "trigger.dev dev",
.env.example (1)
1-43
: Minor.env
lint nitsTrailing whitespace (line 8) and several unordered keys flagged by
dotenv-linter
won’t break runtime but keeping the file tidy avoids churn in future diffs.🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 4-4: [UnorderedKey] The NEXT_PUBLIC_SUPABASE_ANON_KEY key should go before the NEXT_PUBLIC_SUPABASE_URL key
[warning] 8-8: [TrailingWhitespace] Trailing whitespace detected
[warning] 9-9: [UnorderedKey] The ANTHROPIC_API_KEY key should go before the OPENAI_API_KEY key
[warning] 10-10: [UnorderedKey] The GOOGLE_GENERATIVE_AI_API_KEY key should go before the OPENAI_API_KEY key
[warning] 14-14: [UnorderedKey] The GOOGLE_VERTEX_LOCATION key should go before the GOOGLE_VERTEX_PROJECT key
[warning] 15-15: [UnorderedKey] The GOOGLE_APPLICATION_CREDENTIALS key should go before the GOOGLE_VERTEX_LOCATION key
[warning] 24-24: [UnorderedKey] The AWS_ACCESS_KEY_ID key should go before the AWS_REGION key
[warning] 27-27: [UnorderedKey] The SOURCE_EMAIL key should go before the SOURCE_NAME key
[warning] 30-30: [UnorderedKey] The DEEPGRAM_API_KEY key should go before the DEEPGRAM_ENV key
[warning] 40-40: [UnorderedKey] The AURORA_EXTERNAL_URL key should go before the SCRAPER_URL key
[warning] 41-41: [UnorderedKey] The AURORA_EXTERNAL_WSID key should go before the SCRAPER_URL key
[warning] 42-42: [UnorderedKey] The PROXY_API_KEY key should go before the SCRAPER_URL key
[warning] 43-43: [EndingBlankLine] No blank line at the end of the file
[warning] 43-43: [UnorderedKey] The NEXT_PUBLIC_PROXY_API_KEY key should go before the PROXY_API_KEY key
packages/trigger/example.ts (1)
3-16
: Add typing & explicit return for stronger contractsRight now both
payload
and the returned object areany
. Giving them real types makes the task safer to evolve and helps IDEs.-export const helloWorldTask = task({ +interface HelloPayload { + name: string; +} + +interface HelloResult { + message: string; +} + +export const helloWorldTask = task<HelloPayload, HelloResult>({ id: 'hello-world', maxDuration: 300, - run: async (payload: any, { ctx }) => { + run: async (payload, { ctx }) => {No behaviour change, compile-time safety up.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 3-4: packages/trigger/example.ts#L3-L4
Added lines #L3 - L4 were not covered by tests
[warning] 6-8: packages/trigger/example.ts#L6-L8
Added lines #L6 - L8 were not covered by tests
[warning] 10-10: packages/trigger/example.ts#L10
Added line #L10 was not covered by tests
[warning] 12-16: packages/trigger/example.ts#L12-L16
Added lines #L12 - L16 were not covered by tests.gitignore (1)
30-48
: Duplicate entries & ordering
.env.local
now appears twice and.env.*.local
already covers most cases. Consolidating keeps the file shorter and avoids future merge conflicts.-.env.local ... -.env.local -.env.*.local +.env.local +.env.*.localNot critical, just housekeeping.
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/active-sync.tsx (1)
94-94
: Pass the handler reference directly
onClick={() => triggerHelloWorld()}
creates a new closure on every render.
Simply pass the function reference:-<Button onClick={() => triggerHelloWorld()}>Trigger Hello World</Button> +<Button onClick={triggerHelloWorld}>Trigger Hello World</Button>🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 94-94: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/active-sync.tsx#L94
Added line #L94 was not covered by testspackages/trigger/first-scheduled-task.ts (1)
12-34
: Return a value or explicitlyvoid
for clarityTrigger.dev tasks may return a value that becomes the run’s result.
If nothing meaningful is returned consider:- run: async (payload) => { + run: async (payload): Promise<void> => {This documents intent and avoids “implicitly returns
any
” warnings in strict projects.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 12-12: packages/trigger/first-scheduled-task.ts#L12
Added line #L12 was not covered by tests
[warning] 16-16: packages/trigger/first-scheduled-task.ts#L16
Added line #L16 was not covered by tests
[warning] 21-21: packages/trigger/first-scheduled-task.ts#L21
Added line #L21 was not covered by tests
[warning] 25-25: packages/trigger/first-scheduled-task.ts#L25
Added line #L25 was not covered by tests
[warning] 30-30: packages/trigger/first-scheduled-task.ts#L30
Added line #L30 was not covered by testspackages/trigger/google-calendar-background-sync.ts (4)
42-56
: Duplicate colour map – import the existing source of truthA very similar colour map already exists in
packages/ui/src/components/ui/legacy/calendar/settings/color-picker.tsx
.
Keeping two independent copies risks divergence. Export the original map
and reuse it here (or move both to a shared util).🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 42-56: packages/trigger/google-calendar-background-sync.ts#L42-L56
Added lines #L42 - L56 were not covered by tests
79-85
: Unnecessary| null
union complicates the loop
googleTokens
is always an array, so the extra| null
union forces
defensivetokens || []
logic later. Simply type it asToken[]
.-const tokens = googleTokens as { +type GoogleToken = { ws_id: string; access_token: string; refresh_token: string; -}[] | null; +}[]; +const tokens: GoogleToken[] = googleTokens;
135-138
:onConflict: 'google_event_id'
may cause cross-workspace collisionsThe same Google event ID can exist in multiple workspaces. Consider a
composite unique key(ws_id, google_event_id)
to avoid accidental
overwrites between tenants.
105-113
: Pagination not handled – events beyond 1000 are ignored
calendar.events.list
returns anextPageToken
when there are more than
maxResults
. Loop until the token is empty to capture all events, or
lowermaxResults
and page through.packages/types/src/supabase.ts (2)
7334-7342
: Unnecessary union overload – prefer a singleArgs
shape with optional props
count_search_users
now exposes two overlapping object shapes that differ only by the additional optional props (role_filter
,enabled_filter
).
Because both members share the same required key (search_query
), the resulting union collapses the discriminant and effectively widens every optional field anyway, losing most of the intended strictness while introducing type-noise.-Args: - | { search_query: string } - | { - search_query: string; - role_filter?: string; - enabled_filter?: boolean; - }; +Args: { + search_query: string; + role_filter?: string; + enabled_filter?: boolean; +};This keeps the public surface identical while reducing code size and compile-time complexity.
Same pattern reappears in other functions below (e.g.search_users
,generate_cross_app_token
).
7748-7756
: Duplicate overload pattern appears again – consider consolidating
search_users
repeats the two-branch union with the same downside described above.
You can safely fold the variants into a single definition with all truly optional fields marked optional.
Maintaining consistency across the generated types will avoid confusion for consumers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lock
is excluded by!**/*.lock
📒 Files selected for processing (12)
.env.example
(1 hunks).gitignore
(2 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/active-sync.tsx
(2 hunks)apps/web/src/app/api/hello-world/route.ts
(1 hunks)package.json
(1 hunks)packages/trigger/example.ts
(1 hunks)packages/trigger/first-scheduled-task.ts
(1 hunks)packages/trigger/google-calendar-background-sync.ts
(1 hunks)packages/trigger/package.json
(1 hunks)packages/types/src/supabase.ts
(17 hunks)trigger.config.ts
(1 hunks)turbo.json
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/web/src/app/api/hello-world/route.ts (1)
packages/trigger/example.ts (1)
helloWorldTask
(3-16)
packages/trigger/google-calendar-background-sync.ts (1)
packages/ui/src/components/ui/legacy/calendar/settings/color-picker.tsx (1)
colorMap
(8-88)
🪛 GitHub Check: codecov/patch
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/active-sync.tsx
[warning] 57-62: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/active-sync.tsx#L57-L62
Added lines #L57 - L62 were not covered by tests
[warning] 94-94: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/active-sync.tsx#L94
Added line #L94 was not covered by tests
packages/trigger/example.ts
[warning] 3-4: packages/trigger/example.ts#L3-L4
Added lines #L3 - L4 were not covered by tests
[warning] 6-8: packages/trigger/example.ts#L6-L8
Added lines #L6 - L8 were not covered by tests
[warning] 10-10: packages/trigger/example.ts#L10
Added line #L10 was not covered by tests
[warning] 12-16: packages/trigger/example.ts#L12-L16
Added lines #L12 - L16 were not covered by tests
apps/web/src/app/api/hello-world/route.ts
[warning] 2-2: apps/web/src/app/api/hello-world/route.ts#L2
Added line #L2 was not covered by tests
[warning] 4-4: apps/web/src/app/api/hello-world/route.ts#L4
Added line #L4 was not covered by tests
[warning] 9-13: apps/web/src/app/api/hello-world/route.ts#L9-L13
Added lines #L9 - L13 were not covered by tests
[warning] 15-16: apps/web/src/app/api/hello-world/route.ts#L15-L16
Added lines #L15 - L16 were not covered by tests
packages/trigger/google-calendar-background-sync.ts
[warning] 2-4: packages/trigger/google-calendar-background-sync.ts#L2-L4
Added lines #L2 - L4 were not covered by tests
[warning] 6-8: packages/trigger/google-calendar-background-sync.ts#L6-L8
Added lines #L6 - L8 were not covered by tests
[warning] 10-16: packages/trigger/google-calendar-background-sync.ts#L10-L16
Added lines #L10 - L16 were not covered by tests
[warning] 18-21: packages/trigger/google-calendar-background-sync.ts#L18-L21
Added lines #L18 - L21 were not covered by tests
[warning] 23-26: packages/trigger/google-calendar-background-sync.ts#L23-L26
Added lines #L23 - L26 were not covered by tests
[warning] 28-28: packages/trigger/google-calendar-background-sync.ts#L28
Added line #L28 was not covered by tests
[warning] 31-36: packages/trigger/google-calendar-background-sync.ts#L31-L36
Added lines #L31 - L36 were not covered by tests
[warning] 38-40: packages/trigger/google-calendar-background-sync.ts#L38-L40
Added lines #L38 - L40 were not covered by tests
[warning] 42-56: packages/trigger/google-calendar-background-sync.ts#L42-L56
Added lines #L42 - L56 were not covered by tests
[warning] 58-59: packages/trigger/google-calendar-background-sync.ts#L58-L59
Added lines #L58 - L59 were not covered by tests
[warning] 61-63: packages/trigger/google-calendar-background-sync.ts#L61-L63
Added lines #L61 - L63 were not covered by tests
[warning] 65-66: packages/trigger/google-calendar-background-sync.ts#L65-L66
Added lines #L65 - L66 were not covered by tests
packages/trigger/first-scheduled-task.ts
[warning] 3-5: packages/trigger/first-scheduled-task.ts#L3-L5
Added lines #L3 - L5 were not covered by tests
[warning] 7-9: packages/trigger/first-scheduled-task.ts#L7-L9
Added lines #L7 - L9 were not covered by tests
[warning] 12-12: packages/trigger/first-scheduled-task.ts#L12
Added line #L12 was not covered by tests
[warning] 16-16: packages/trigger/first-scheduled-task.ts#L16
Added line #L16 was not covered by tests
[warning] 21-21: packages/trigger/first-scheduled-task.ts#L21
Added line #L21 was not covered by tests
[warning] 25-25: packages/trigger/first-scheduled-task.ts#L25
Added line #L25 was not covered by tests
[warning] 30-30: packages/trigger/first-scheduled-task.ts#L30
Added line #L30 was not covered by tests
[warning] 33-35: packages/trigger/first-scheduled-task.ts#L33-L35
Added lines #L33 - L35 were not covered by tests
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 4-4: [UnorderedKey] The NEXT_PUBLIC_SUPABASE_ANON_KEY key should go before the NEXT_PUBLIC_SUPABASE_URL key
[warning] 8-8: [TrailingWhitespace] Trailing whitespace detected
[warning] 9-9: [UnorderedKey] The ANTHROPIC_API_KEY key should go before the OPENAI_API_KEY key
[warning] 10-10: [UnorderedKey] The GOOGLE_GENERATIVE_AI_API_KEY key should go before the OPENAI_API_KEY key
[warning] 14-14: [UnorderedKey] The GOOGLE_VERTEX_LOCATION key should go before the GOOGLE_VERTEX_PROJECT key
[warning] 15-15: [UnorderedKey] The GOOGLE_APPLICATION_CREDENTIALS key should go before the GOOGLE_VERTEX_LOCATION key
[warning] 24-24: [UnorderedKey] The AWS_ACCESS_KEY_ID key should go before the AWS_REGION key
[warning] 27-27: [UnorderedKey] The SOURCE_EMAIL key should go before the SOURCE_NAME key
[warning] 30-30: [UnorderedKey] The DEEPGRAM_API_KEY key should go before the DEEPGRAM_ENV key
[warning] 40-40: [UnorderedKey] The AURORA_EXTERNAL_URL key should go before the SCRAPER_URL key
[warning] 41-41: [UnorderedKey] The AURORA_EXTERNAL_WSID key should go before the SCRAPER_URL key
[warning] 42-42: [UnorderedKey] The PROXY_API_KEY key should go before the SCRAPER_URL key
[warning] 43-43: [EndingBlankLine] No blank line at the end of the file
[warning] 43-43: [UnorderedKey] The NEXT_PUBLIC_PROXY_API_KEY key should go before the PROXY_API_KEY key
🔇 Additional comments (4)
package.json (1)
64-70
: Dependency hygiene
concurrently@^9.1.2
is ~4 years old and does not officially list Node 20 in its engines field. Since the project requiresnode>=20
, bumping to^8.0.1
(LTS) or the latest v8/9 minor alleviates potential ESM/Signal handling quirks seen on Node 20.trigger.config.ts (1)
4-6
: Check in secrets only when non-sensitive
project: 'proj_uoblreqcnhwrarddxrds'
appears hard-coded.
If this ID is considered sensitive in Trigger.dev (e.g. ties to a private workspace) move it to an environment variable (process.env.TRIGGER_PROJECT_ID
) to allow safer open-source sharing.packages/types/src/supabase.ts (2)
7408-7414
: Verify function return columns against the actual SQL view
get_hourly_prompt_completion_tokens
’s return signature now liststotal_prompt_tokens
&total_completion_tokens
.
Make sure the underlying SQL (or Supabase RPC) really selects those exact snake-case columns; otherwise callers will compile but fail at runtime because the fields arenull
/ missing.
1-20
: Auto-generated file – avoid manual edits
supabase.ts
is normally produced bysupabase gen types
(or the Supabase VS Code extension).
Manual tweaks tend to be overwritten and may cause merge noise like this 8 000-line diff.
If these changes come from the generator, all good.
If not, consider regenerating instead of hand-editing to keep the file authoritative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/trigger/package.json (2)
6-7
: Fix entry points to compiled JavaScript and type declarations
The"main"
and"types"
fields currently point to TS files, which breaks plain Node resolution at runtime. Please update to reference compiled output (e.g.,dist/google-calendar-background-sync.js
anddist/google-calendar-background-sync.d.ts
) and add a build step to generate these.- "main": "./google-calendar-background-sync.ts", - "types": "./google-calendar-background-sync.ts", + "main": "./dist/google-calendar-background-sync.js", + "types": "./dist/google-calendar-background-sync.d.ts",
8-17
: Align exports with compiled bundles
Theexports
entries still reference TS sources; they should point to the transpiled.js
and.d.ts
indist/
.- "exports": { - ".": { - "types": "./google-calendar-background-sync.ts", - "import": "./google-calendar-background-sync.ts" - }, - "./google-calendar-background-sync": { - "types": "./google-calendar-background-sync.ts", - "import": "./google-calendar-background-sync.ts" - } - }, + "exports": { + ".": { + "types": "./dist/google-calendar-background-sync.d.ts", + "import": "./dist/google-calendar-background-sync.js" + }, + "./google-calendar-background-sync": { + "types": "./dist/google-calendar-background-sync.d.ts", + "import": "./dist/google-calendar-background-sync.js" + } + },
🧹 Nitpick comments (1)
packages/trigger/package.json (1)
2-4
: Enhance package metadata
Addingdescription
,repository
, andauthor
fields will improve clarity and traceability, even for private packages."name": "@tuturuuu/trigger", + "description": "Background sync tasks for Google Calendar using Trigger.dev", + "repository": "https://github.com/tutur3u/platform.git", + "author": "DennieDan", "version": "0.1.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
package.json
(1 hunks)packages/trigger/google-calendar-background-sync.ts
(1 hunks)packages/trigger/package.json
(1 hunks)packages/trigger/trigger.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- package.json
- packages/trigger/trigger.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/trigger/google-calendar-background-sync.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/trigger/package.json (2)
4-5
: Add a build script for transpilation
Without ascripts.build
command, TypeScript sources won’t be compiled to JS before usage, causing runtime errors.
Apply this diff to introduce a build step using the Trigger.dev build tool:"private": true, "type": "module", + "scripts": { + "build": "trigger.dev build" + }, "main": "./example.ts",
6-7
: Use compiled JavaScript for themain
andtypes
entry points
Pointing both fields at.ts
files breaks runtime resolution in Node.js. They must reference transpiled outputs:- "main": "./example.ts", - "types": "./example.ts", + "main": "./dist/example.js", + "types": "./dist/example.d.ts",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/trigger/package.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Prettier Check (23)
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- GitHub Check: Deploy-Preview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (5)
packages/trigger/google-calendar-background-sync.ts (5)
20-22
: Avoid logging secrets / configuration values (duplicate of previous review)
Thisconsole.log
leaksNEXT_PUBLIC_SUPABASE_URL
into Trigger.dev logs. Please remove or guard it behind a safe DEBUG flag.
24-27
: Validate env-vars instead of using non-null (!
) assertions (duplicate)
The previous review suggested a fail-fast check – still relevant.
67-70
: Filter out rows withNULL
access tokens at the SQL level (duplicate)
Fetching useless rows wastes bandwidth and CPU. Add.not('access_token', 'is', null)
to the query.
95-101
: Skip iteration when access token is missing (duplicate)Add
continue;
after the error log to avoid calling Google APIs with empty creds.
136-139
: Remove full access / refresh tokens from logs (duplicate & security)
Tokens are credentials; logging them is a critical leak.
🧹 Nitpick comments (8)
packages/ui/src/hooks/use-calendar-sync.tsx (1)
574-576
: Reuse the existing Supabase instance to avoid an extra TCP handshake
updateLastUpsert
creates a new client when none is supplied, generating an extra connection per sync.
Pass the already-initialisedsupabase
variable:-await updateLastUpsert(wsId); +await updateLastUpsert(wsId, supabase);Not critical, but saves one connection round-trip and keeps the open-connection count predictable.
apps/db/supabase/migrations/20250101000000_add_calendar_sync_coordination.sql (1)
18-23
: RLS policy grants all commands – restrict if not needed
FOR ALL
allowsselect, insert, update, delete
.
If the UI never deletes coordination rows, consider limiting the policy to the required commands to narrow the attack surface:-- e.g. AS PERMISSIVE FOR SELECT, UPDATE TO authenticated ...packages/utils/src/calendar-sync-coordination.md (6)
14-25
: Consider linking schema to migration script
Instead of inlining the same SQL DDL here, reference the actual migration file (apps/db/supabase/migrations/20250101000000_add_calendar_sync_coordination.sql
). This avoids duplication and keeps the docs in sync with the source.
49-68
: Add missing article “the” for consistency
Update the bullet to read:
true
if the date range overlaps with 4 weeks from the current week
This aligns with the phrasing elsewhere.
69-87
: ClarifyFOUR_WEEKS_FROM_CURRENT_WEEK
units
The constant name suggests weeks but is used as days (28). Consider renaming toFOUR_WEEKS_IN_DAYS
or explicitly note in the description that its value represents 28 days.
89-129
: Clarify variables in Active Sync snippet
The example refers todates
andwsId
without context. Add a brief note or comment indicating thatdates
is the current view’s date array andwsId
is the workspace identifier.
131-162
: Definetokens
shape in Background Sync snippet
The loop iterates overtokens
but their structure isn’t detailed. Mention that eachtoken
entry must includews_id
(and any auth credentials) so readers know what to pass.
173-187
: Consolidate migration and schema sections
The migration SQL duplicates the Database Schema example. Consider merging these sections or linking to the migration file only once. Also verify the SQL code fence is properly closed to render correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/db/supabase/migrations/20250101000000_add_calendar_sync_coordination.sql
(1 hunks)packages/trigger/google-calendar-background-sync.ts
(1 hunks)packages/ui/src/hooks/use-calendar-sync.tsx
(3 hunks)packages/utils/src/calendar-sync-coordination.md
(1 hunks)packages/utils/src/calendar-sync-coordination.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/ui/src/hooks/use-calendar-sync.tsx (1)
packages/utils/src/calendar-sync-coordination.ts (2)
canProceedWithSync
(14-83)updateLastUpsert
(117-142)
packages/trigger/google-calendar-background-sync.ts (2)
packages/ui/src/components/ui/legacy/calendar/settings/color-picker.tsx (1)
colorMap
(8-88)packages/utils/src/calendar-sync-coordination.ts (2)
BACKGROUND_SYNC_RANGE
(6-6)updateLastUpsert
(117-142)
🪛 GitHub Check: codecov/patch
packages/trigger/google-calendar-background-sync.ts
[warning] 2-3: packages/trigger/google-calendar-background-sync.ts#L2-L3
Added lines #L2 - L3 were not covered by tests
[warning] 7-10: packages/trigger/google-calendar-background-sync.ts#L7-L10
Added lines #L7 - L10 were not covered by tests
[warning] 12-14: packages/trigger/google-calendar-background-sync.ts#L12-L14
Added lines #L12 - L14 were not covered by tests
[warning] 16-22: packages/trigger/google-calendar-background-sync.ts#L16-L22
Added lines #L16 - L22 were not covered by tests
[warning] 24-27: packages/trigger/google-calendar-background-sync.ts#L24-L27
Added lines #L24 - L27 were not covered by tests
[warning] 29-32: packages/trigger/google-calendar-background-sync.ts#L29-L32
Added lines #L29 - L32 were not covered by tests
[warning] 34-34: packages/trigger/google-calendar-background-sync.ts#L34
Added line #L34 was not covered by tests
[warning] 37-42: packages/trigger/google-calendar-background-sync.ts#L37-L42
Added lines #L37 - L42 were not covered by tests
[warning] 44-46: packages/trigger/google-calendar-background-sync.ts#L44-L46
Added lines #L44 - L46 were not covered by tests
[warning] 48-62: packages/trigger/google-calendar-background-sync.ts#L48-L62
Added lines #L48 - L62 were not covered by tests
[warning] 64-65: packages/trigger/google-calendar-background-sync.ts#L64-L65
Added lines #L64 - L65 were not covered by tests
[warning] 67-69: packages/trigger/google-calendar-background-sync.ts#L67-L69
Added lines #L67 - L69 were not covered by tests
🪛 LanguageTool
packages/utils/src/calendar-sync-coordination.md
[uncategorized] ~12-~12: The preposition ‘to’ seems more likely in this position.
Context: ...tencies from concurrent updates - Syncs on date ranges too far in the past or futu...
(AI_HYDRA_LEO_REPLACE_ON_TO)
[uncategorized] ~35-~35: Loose punctuation mark.
Context: ...en workspace. Parameters: - wsId
: The workspace ID - supabase
: Optional...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~55-~55: Loose punctuation mark.
Context: ...nt week. Parameters: - startDate
: Start date to check - endDate
: End da...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~60-~60: You might be missing the article “the” here.
Context: ...e date range overlaps with 4 weeks from current week - false
if the date range is out...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~75-~75: Loose punctuation mark.
Context: ...nc operation. Parameters: - wsId
: The workspace ID - supabase
: Optional...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
🔇 Additional comments (5)
packages/utils/src/calendar-sync-coordination.ts (1)
95-109
: 4-week window only covers current→future, not past
isWithinBackgroundSyncRange
requiresend
to be after the start of the current week, so any view entirely in the past (e.g. last month) is always “out of range”, even though the comment says “4 weeks from current week”.
Confirm whether past weeks should be included and adjust the logic if necessary (e.g. subtractBACKGROUND_SYNC_RANGE
fromstartOfCurrentWeek
).packages/utils/src/calendar-sync-coordination.md (4)
1-13
: Introduction section looks solid
The overview clearly states the module’s purpose and highlights key concerns; no issues found.
27-48
:canProceedWithSync
doc is clear
Parameters and behavior are well-defined, including the error-fallback logic. No changes needed here.
164-172
: Error handling description is accurate
The fault-tolerant strategy and logging fallback are well-documented; no changes needed.
189-198
: Benefits section effectively summarizes advantages
Clear, concise, and captures all key points.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/trigger/.env.example (2)
8-8
: Remove trailing whitespaceThere's an extra space at the end of this line. Please remove it to keep the file clean.
Proposed diff:
-OPENAI_API_KEY=YOUR_OPENAI_API_KEY +OPENAI_API_KEY=YOUR_OPENAI_API_KEY
43-43
: Ensure newline at end of filePOSIX standards recommend a trailing newline. Please add a blank line at the end.
Proposed diff:
NEXT_PUBLIC_PROXY_API_KEY=YOUR_NEXT_PUBLIC_PROXY_API_KEY +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/trigger/.env.example
(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
packages/trigger/.env.example
[warning] 4-4: [UnorderedKey] The NEXT_PUBLIC_SUPABASE_ANON_KEY key should go before the NEXT_PUBLIC_SUPABASE_URL key
[warning] 8-8: [TrailingWhitespace] Trailing whitespace detected
[warning] 9-9: [UnorderedKey] The ANTHROPIC_API_KEY key should go before the OPENAI_API_KEY key
[warning] 10-10: [UnorderedKey] The GOOGLE_GENERATIVE_AI_API_KEY key should go before the OPENAI_API_KEY key
[warning] 14-14: [UnorderedKey] The GOOGLE_VERTEX_LOCATION key should go before the GOOGLE_VERTEX_PROJECT key
[warning] 15-15: [UnorderedKey] The GOOGLE_APPLICATION_CREDENTIALS key should go before the GOOGLE_VERTEX_LOCATION key
[warning] 24-24: [UnorderedKey] The AWS_ACCESS_KEY_ID key should go before the AWS_REGION key
[warning] 27-27: [UnorderedKey] The SOURCE_EMAIL key should go before the SOURCE_NAME key
[warning] 30-30: [UnorderedKey] The DEEPGRAM_API_KEY key should go before the DEEPGRAM_ENV key
[warning] 40-40: [UnorderedKey] The AURORA_EXTERNAL_URL key should go before the SCRAPER_URL key
[warning] 41-41: [UnorderedKey] The AURORA_EXTERNAL_WSID key should go before the SCRAPER_URL key
[warning] 42-42: [UnorderedKey] The PROXY_API_KEY key should go before the SCRAPER_URL key
[warning] 43-43: [EndingBlankLine] No blank line at the end of the file
[warning] 43-43: [UnorderedKey] The NEXT_PUBLIC_PROXY_API_KEY key should go before the PROXY_API_KEY key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/db/supabase/migrations/20250616000000_add_calendar_sync_coordination.sql (3)
1-7
: Add NOT NULL to timestamp columns for consistency
The defaultnow()
ensures values, but markingcreated_at
andupdated_at
asNOT NULL
prevents accidental nulls and enforces data integrity.
21-35
: Consider splitting the policy per operation
A unifiedFOR ALL
policy works, but defining separateFOR SELECT
(using) andFOR INSERT/UPDATE
(with check) policies can improve clarity and make future adjustments safer.
46-51
: Schema-qualify the trigger target
Specifypublic.workspace_calendar_sync_coordination
in theCREATE TRIGGER
statement to avoid ambiguity if thesearch_path
changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
apps/db/supabase/migrations/20250616000000_add_calendar_sync_coordination.sql
(1 hunks)package.json
(1 hunks)packages/types/src/supabase.ts
(17 hunks)turbo.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- turbo.json
- packages/types/src/supabase.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (4)
apps/db/supabase/migrations/20250616000000_add_calendar_sync_coordination.sql (4)
9-12
: Primary key constraint looks correct
Definingws_id
as the primary key ensures one coordination record per workspace.
13-17
: Foreign key with cascade delete is appropriate
TheON DELETE CASCADE
ensures coordination rows are cleaned up when a workspace is removed.
18-20
: Row-Level Security enabled correctly
RLS is turned on, which is required before any policies are applied.
37-44
: Trigger function implementation is solid
The function correctly setsNEW.updated_at = now()
on each update.
apps/db/supabase/migrations/20250616000000_add_calendar_sync_coordination.sql
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Will merge after a quick demo.
Notes:
TRIGGER_SECRET_KEY
, supabase keys, google client ids must be in.env
in the root directoryConnect local dev to server
Deploy command
Current Warning
Remaining Tasks
trigger
in rootpackage.json
+.env.local
topackage.json
insidepackages/
Summary by CodeRabbit