fix:Ensure seamless onboarding-to-dashboard navigation with consistent, flicker-free experience#28693
Draft
Arjun3492 wants to merge 2528 commits intocalcom:mainfrom
Conversation
401 unified cal view dev
…oading-on-dashboard-booking-and-team-page fix: idenfying public page hook
chore(unified-calendar):remove message 'No events in this range.'
401 unified cal view dev
…iew-and-redirect-or-block-google-auth 988 improve detect in app webview and redirect or block google auth
…iew-and-redirect-or-block-google-auth chore:improved auto browser redirect strategy for supported agents in…
…iew-and-redirect-or-block-google-auth fix:webview blocker conditional redering
…iew-and-redirect-or-block-google-auth 988 improve detect in app webview and redirect or block google auth
…iew-and-redirect-or-block-google-auth chore:show divider separator in case of webview too
…iew-and-redirect-or-block-google-auth chore:show didvider separator in case of webview too
React awesome query version mismatch fix
…e-naming-description-translations-for-better-clarity chore(event-type):Improve “Look Busy” feature copy and translations for better clarity
…e-configuration-for-enterprise
…am member impersonation
…r-stoping-team-impersonation enh(teams):Fix team impersonation stop flow to restore original session instead of logout
…schedule-url-in-webhook-data enh(webhook):Include `cancellationUrl` and `rescheduleUrl` in booking webhooks for confirmed create + reschedule flows
…te-limiting-locking-and-otp-cooldowns pvt:enh(sms): prevent sms abuse via rate limiting locking and otp cooldowns
…uration-for-enterprise feat(Enterprise booking page): booking page configuration for enterprise
… duplicate client redirects
Contributor
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
|
|
github-actions
bot
added
❗️ migrations
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
38 issues found across 2442 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/connector/src/repositories/oauth-client.repository.ts">
<violation number="1" location="apps/connector/src/repositories/oauth-client.repository.ts:36">
P2: Custom agent: **Enforce Singular Naming for Single-Item Functions**
Rename this single-item fetcher to a singular form (e.g., `getByEventTypeHost`) to match its single-result behavior and comply with the singular naming rule.</violation>
</file>
<file name="apps/connector/src/routes/public/eventtypes.route.ts">
<violation number="1" location="apps/connector/src/routes/public/eventtypes.route.ts:161">
P2: `"code" in (error as object)` can throw if `error` is null or a primitive. Guard for a non-null object before using the `in` operator to avoid errors inside the catch block and preserve the intended 404 mapping.</violation>
</file>
<file name="apps/connector/src/auth/guards.ts">
<violation number="1" location="apps/connector/src/auth/guards.ts:47">
P0: Custom agent: **Avoid Logging Sensitive Information**
Logging `request.headers` exposes the full `Authorization` header, which contains the Bearer token (API key or access token). Remove `headers` from this log or redact sensitive headers before logging.</violation>
<violation number="2" location="apps/connector/src/auth/guards.ts:79">
P2: authenticateOptional never catches UnauthorizedError because it wraps only guard creation; the returned authenticateBearer handler still throws on missing/invalid credentials, so auth is not actually optional.</violation>
<violation number="3" location="apps/connector/src/auth/guards.ts:118">
P1: Custom agent: **Avoid Logging Sensitive Information**
Logging `request.user.email` exposes PII in application logs. Remove the `email` field or mask it (e.g., only log `userId` for audit purposes).</violation>
</file>
<file name="apps/connector/src/routes/public/schedule.route.ts">
<violation number="1" location="apps/connector/src/routes/public/schedule.route.ts:99">
P2: The delete route fires scheduleService.delete without awaiting its async DB operations, so the success response can be returned before deletion completes and errors can escape the handler.</violation>
<violation number="2" location="apps/connector/src/routes/public/schedule.route.ts:128">
P1: Custom agent: **Avoid Logging Sensitive Information**
Remove the console.log of the updated schedule data to avoid logging potentially sensitive user information.</violation>
</file>
<file name="apps/connector/src/routes/public/booking.route.ts">
<violation number="1" location="apps/connector/src/routes/public/booking.route.ts:126">
P1: Custom agent: **Avoid Logging Sensitive Information**
Raw error object logged from `handleNewBooking` may contain PII (guest emails, booking details). Log only `error.message` or a sanitized summary instead.
(Based on your team's feedback about not logging raw error objects to avoid exposing sensitive data.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/connector/src/routes/public/booking.route.ts:226">
P2: `arePlatformEmailsEnabled` is always true due to `|| true`, so the `x-cal-disable-emails` header can never disable emails.</violation>
<violation number="3" location="apps/connector/src/routes/public/booking.route.ts:310">
P1: Custom agent: **Avoid Logging Sensitive Information**
Raw error object logged from `confirmHandler` may contain sensitive user/booking data. Log only `error.message` instead of the full error object.
(Based on your team's feedback about not logging raw error objects to avoid exposing sensitive data.) [FEEDBACK_USED]</violation>
<violation number="4" location="apps/connector/src/routes/public/booking.route.ts:343">
P1: Reassign handler loads booking by raw ID without verifying the booking belongs to the authenticated user (bookingExists check). This allows mutation attempts on other users’ bookings if an ID is guessed.</violation>
<violation number="5" location="apps/connector/src/routes/public/booking.route.ts:380">
P2: Reschedule creates a new booking and cancels the original, but the handler re-reads by the original id, so the response can return the cancelled booking instead of the new one.</violation>
</file>
<file name="apps/connector/src/auth/strategies/api-key.strategy.ts">
<violation number="1" location="apps/connector/src/auth/strategies/api-key.strategy.ts:116">
P2: Expiration check truncates time-of-day, allowing keys to remain valid until the next day even if expiresAt has a specific time.</violation>
</file>
<file name="apps/connector/src/repositories/schedule.repository.ts">
<violation number="1" location="apps/connector/src/repositories/schedule.repository.ts:66">
P2: `detachDefaultScheduleFromUsers` uses `defaultScheduleId: undefined`, which Prisma treats as “no update,” so the default schedule likely isn’t cleared. The field is nullable and other code clears it with `null`, so this should use `null` to detach.</violation>
</file>
<file name="apps/api/v2/README.md">
<violation number="1" location="apps/api/v2/README.md:89">
P2: Code fence is closed with four backticks instead of three, leaving the ```bash block unclosed and breaking README rendering past this point.</violation>
</file>
<file name="apps/connector/src/repositories/user.repository.ts">
<violation number="1" location="apps/connector/src/repositories/user.repository.ts:156">
P2: emailExists passes a string excludeUserId directly to the Prisma id filter, but User.id is an Int. Callers pass id.toString(), so this can cause Prisma validation errors or fail to exclude the current user during email checks. Convert to a number before filtering.</violation>
<violation number="2" location="apps/connector/src/repositories/user.repository.ts:169">
P1: Custom agent: **Avoid Logging Sensitive Information**
Remove the console log of user data. Logging user lookup results can expose PII in application logs.</violation>
</file>
<file name=".github/workflows/main-deploy.yml">
<violation number="1" location=".github/workflows/main-deploy.yml:465">
P2: Missing-secret detection does not gate deployment; the deploy step runs even when `check_required_secrets_status` sets `passed=failure`, so the workflow can attempt SSH with empty secrets/outputs.</violation>
<violation number="2" location=".github/workflows/main-deploy.yml:465">
P2: Deploy runs unconditionally even though the Docker build step is allowed to fail, so EC2 deployment can proceed without a valid image (e.g., build/push failed and image doesn’t exist). Reinstate a guard so deploy only runs when a build succeeded or an existing image is available.</violation>
<violation number="3" location=".github/workflows/main-deploy.yml:517">
P0: Custom agent: **Avoid Logging Sensitive Information**
The `exec_command` function logs the full command text at DEBUG level (`log_message "DEBUG" "Command: $command_to_run"`), which will write AWS credentials (`AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`) and `DATABASE_URL` (containing DB password) into the log file whenever those commands execute. This log file is later emailed via SendGrid, exposing secrets in plaintext over email.
Remove or redact the DEBUG log line, or at minimum do not pass secrets as inline arguments to `exec_command`.</violation>
</file>
<file name=".github/ISSUE_TEMPLATE/feature_request.md">
<violation number="1" location=".github/ISSUE_TEMPLATE/feature_request.md:5">
P2: Feature request template no longer applies the `🚨 needs approval` label required by CONTRIBUTING.md, which can bypass the documented approval workflow for new feature requests.</violation>
</file>
<file name="apps/connector/src/routes/public/availability.route.ts">
<violation number="1" location="apps/connector/src/routes/public/availability.route.ts:85">
P2: Delete is async but not awaited; route may return success before deletion completes and errors won’t propagate.</violation>
<violation number="2" location="apps/connector/src/routes/public/availability.route.ts:113">
P2: Custom agent: **Avoid Logging Sensitive Information**
Remove logging of the full availability update payload; it may include sensitive user data.</violation>
</file>
<file name="apps/connector/src/routes/health.ts">
<violation number="1" location="apps/connector/src/routes/health.ts:26">
P2: Response schema includes data.version but the handler never provides it, so the documented field is never returned.</violation>
<violation number="2" location="apps/connector/src/routes/health.ts:39">
P2: Health route bypasses centralized env config by reading `process.env.NODE_ENV` directly with an inline fallback.</violation>
</file>
<file name="apps/connector/src/auth/strategies/access-token.strategy.ts">
<violation number="1" location="apps/connector/src/auth/strategies/access-token.strategy.ts:61">
P1: Team owner fallback authenticates as an arbitrary OWNER because `take: 1` is used without deterministic ordering.</violation>
</file>
<file name="apps/connector/src/routes/admin/admin-user.route.ts">
<violation number="1" location="apps/connector/src/routes/admin/admin-user.route.ts:110">
P1: Custom agent: **Avoid Logging Sensitive Information**
Remove raw user payload logging. This logs PII-bearing user data (including email) and violates the Avoid Logging Sensitive Information rule.</violation>
<violation number="2" location="apps/connector/src/routes/admin/admin-user.route.ts:277">
P2: `"code" in (error as object)` can throw in the catch block when a non-object is thrown, masking the original error. Guard the second branch with a non-null object check like the first branch.</violation>
</file>
<file name="apps/connector/src/repositories/booking.repository.ts">
<violation number="1" location="apps/connector/src/repositories/booking.repository.ts:129">
P1: `deleteBookingById` swallows all delete errors and returns `null`, masking real database failures as not-found outcomes.</violation>
</file>
<file name=".github/workflows/i18n.yml">
<violation number="1" location=".github/workflows/i18n.yml:3">
P2: Switching the workflow trigger to `pull_request` (closed) while still relying on repo secrets means merged fork PRs won’t have `GH_ACCESS_TOKEN`/`CI_LINGO_DOT_DEV_API_KEY`, so i18n automation will fail for fork contributions. The repo explicitly supports fork-based PRs, and other workflows note secrets are unavailable for forked PR events.</violation>
</file>
<file name=".github/workflows/worker-deploy.yml">
<violation number="1" location=".github/workflows/worker-deploy.yml:91">
P1: PR-based branch detection uses `github.ref_name`, which can misclassify merged PRs and send main merges down the staging deployment path.</violation>
<violation number="2" location=".github/workflows/worker-deploy.yml:211">
P1: Critical deploy/build failures are masked as non-fatal, allowing the workflow to report success and send misleading success notifications.</violation>
<violation number="3" location=".github/workflows/worker-deploy.yml:462">
P2: Stale worker-new-* containers can block future deployments because startup uses fixed worker-new-N names without pre-cleaning, and the abort guard exits before Phase 4 cleanup when all new workers fail to start. A failed deployment can leave worker-new-* containers that cause name collisions on the next run.</violation>
</file>
<file name="Dockerfile">
<violation number="1" location="Dockerfile:129">
P1: Trailing whitespace after the line-continuation backslash breaks the RUN instruction; the next line will be parsed as a separate Dockerfile instruction and fail the build.</violation>
</file>
<file name="apps/connector/src/plugins/rateLimit.ts">
<violation number="1" location="apps/connector/src/plugins/rateLimit.ts:65">
P2: `result.reset` appears to be an absolute timestamp (tests use `Date.now()`), but this code treats it as a duration by adding `Date.now()` again, inflating `X-RateLimit-Reset` and `Retry-After`.</violation>
</file>
<file name="apps/connector/src/auth/strategies/combined-auth.strategy.ts">
<violation number="1" location="apps/connector/src/auth/strategies/combined-auth.strategy.ts:83">
P1: Custom agent: **Avoid Logging Sensitive Information**
This `console.log` dumps the full error from access token authentication, which may include the bearer token, JWT payload, or other sensitive auth details. Replace with a sanitized log that omits the raw error or use a structured logger that redacts sensitive fields.</violation>
</file>
<file name="apps/connector/src/middlewares/error-handler.ts">
<violation number="1" location="apps/connector/src/middlewares/error-handler.ts:19">
P1: Custom agent: **Avoid Logging Sensitive Information**
Do not log full request headers; they can contain authorization tokens or other sensitive data. Remove headers from the logged payload or explicitly whitelist non-sensitive fields.</violation>
</file>
<file name="apps/connector/src/repositories/base.repository.ts">
<violation number="1" location="apps/connector/src/repositories/base.repository.ts:10">
P1: Custom agent: **Avoid Logging Sensitive Information**
Avoid logging the raw error object here because it can contain sensitive data from database operations. Log only non-sensitive metadata like an error code.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| local command_to_run="$1" | ||
|
|
||
| log_message "INFO" "Executing: $command_name" | ||
| log_message "DEBUG" "Command: $command_to_run" |
Contributor
There was a problem hiding this comment.
P0: Custom agent: Avoid Logging Sensitive Information
The exec_command function logs the full command text at DEBUG level (log_message "DEBUG" "Command: $command_to_run"), which will write AWS credentials (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY) and DATABASE_URL (containing DB password) into the log file whenever those commands execute. This log file is later emailed via SendGrid, exposing secrets in plaintext over email.
Remove or redact the DEBUG log line, or at minimum do not pass secrets as inline arguments to exec_command.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/main-deploy.yml, line 517:
<comment>The `exec_command` function logs the full command text at DEBUG level (`log_message "DEBUG" "Command: $command_to_run"`), which will write AWS credentials (`AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`) and `DATABASE_URL` (containing DB password) into the log file whenever those commands execute. This log file is later emailed via SendGrid, exposing secrets in plaintext over email.
Remove or redact the DEBUG log line, or at minimum do not pass secrets as inline arguments to `exec_command`.</comment>
<file context>
@@ -0,0 +1,1117 @@
+ local command_to_run="$1"
+
+ log_message "INFO" "Executing: $command_name"
+ log_message "DEBUG" "Command: $command_to_run"
+
+ eval "$command_to_run"
</file context>
| error: error instanceof Error ? error.message : error, | ||
| url: request.url, | ||
| method: request.method, | ||
| headers: request.headers, |
Contributor
There was a problem hiding this comment.
P0: Custom agent: Avoid Logging Sensitive Information
Logging request.headers exposes the full Authorization header, which contains the Bearer token (API key or access token). Remove headers from this log or redact sensitive headers before logging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/connector/src/auth/guards.ts, line 47:
<comment>Logging `request.headers` exposes the full `Authorization` header, which contains the Bearer token (API key or access token). Remove `headers` from this log or redact sensitive headers before logging.</comment>
<file context>
@@ -0,0 +1,310 @@
+ error: error instanceof Error ? error.message : error,
+ url: request.url,
+ method: request.method,
+ headers: request.headers,
+ });
+
</file context>
| return ResponseFormatter.notFound(reply, "Booking not found or invalid credentials"); | ||
| } | ||
|
|
||
| const booking = await bookingService.getBookingById(Number(id)); |
Contributor
There was a problem hiding this comment.
P1: Reassign handler loads booking by raw ID without verifying the booking belongs to the authenticated user (bookingExists check). This allows mutation attempts on other users’ bookings if an ID is guessed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/connector/src/routes/public/booking.route.ts, line 343:
<comment>Reassign handler loads booking by raw ID without verifying the booking belongs to the authenticated user (bookingExists check). This allows mutation attempts on other users’ bookings if an ID is guessed.</comment>
<file context>
@@ -0,0 +1,553 @@
+ return ResponseFormatter.notFound(reply, "Booking not found or invalid credentials");
+ }
+
+ const booking = await bookingService.getBookingById(Number(id));
+ if (!booking) {
+ return ResponseFormatter.error(reply, 'Booking not found', 404);
</file context>
| include: { | ||
| members: { | ||
| where: { role: "OWNER" }, | ||
| take: 1, |
Contributor
There was a problem hiding this comment.
P1: Team owner fallback authenticates as an arbitrary OWNER because take: 1 is used without deterministic ordering.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/connector/src/auth/strategies/access-token.strategy.ts, line 61:
<comment>Team owner fallback authenticates as an arbitrary OWNER because `take: 1` is used without deterministic ordering.</comment>
<file context>
@@ -0,0 +1,185 @@
+ include: {
+ members: {
+ where: { role: "OWNER" },
+ take: 1,
+ },
+ },
</file context>
| return deleted; | ||
| } catch (err) { | ||
| // surface not found as null to caller for consistent 404 handling | ||
| return null; |
Contributor
There was a problem hiding this comment.
P1: deleteBookingById swallows all delete errors and returns null, masking real database failures as not-found outcomes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/connector/src/repositories/booking.repository.ts, line 129:
<comment>`deleteBookingById` swallows all delete errors and returns `null`, masking real database failures as not-found outcomes.</comment>
<file context>
@@ -0,0 +1,148 @@
+ return deleted;
+ } catch (err) {
+ // surface not found as null to caller for consistent 404 handling
+ return null;
+ }
+ }
</file context>
| "message" in error && | ||
| typeof (error as any).message === "string" && | ||
| (error as any).message.includes("not found")) || | ||
| ("code" in (error as object) && (error as any).code === "P2025") |
Contributor
There was a problem hiding this comment.
P2: "code" in (error as object) can throw in the catch block when a non-object is thrown, masking the original error. Guard the second branch with a non-null object check like the first branch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/connector/src/routes/admin/admin-user.route.ts, line 277:
<comment>`"code" in (error as object)` can throw in the catch block when a non-object is thrown, masking the original error. Guard the second branch with a non-null object check like the first branch.</comment>
<file context>
@@ -0,0 +1,335 @@
+ "message" in error &&
+ typeof (error as any).message === "string" &&
+ (error as any).message.includes("not found")) ||
+ ("code" in (error as object) && (error as any).code === "P2025")
+ ) {
+ return ResponseFormatter.notFound(reply, "User not found");
</file context>
| name: Run i18n AI automation | ||
| on: | ||
| push: | ||
| pull_request: |
Contributor
There was a problem hiding this comment.
P2: Switching the workflow trigger to pull_request (closed) while still relying on repo secrets means merged fork PRs won’t have GH_ACCESS_TOKEN/CI_LINGO_DOT_DEV_API_KEY, so i18n automation will fail for fork contributions. The repo explicitly supports fork-based PRs, and other workflows note secrets are unavailable for forked PR events.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/i18n.yml, line 3:
<comment>Switching the workflow trigger to `pull_request` (closed) while still relying on repo secrets means merged fork PRs won’t have `GH_ACCESS_TOKEN`/`CI_LINGO_DOT_DEV_API_KEY`, so i18n automation will fail for fork contributions. The repo explicitly supports fork-based PRs, and other workflows note secrets are unavailable for forked PR events.</comment>
<file context>
@@ -1,26 +1,28 @@
name: Run i18n AI automation
on:
- push:
+ pull_request:
+ types: [closed]
branches:
</file context>
| # NOT touch the old workers. They are still serving jobs. Aborting here | ||
| # preserves the running system and avoids an outage. | ||
| # ───────────────────────────────────────────────────────────────────────────── | ||
| if [ "$WORKERS_STARTED" -eq 0 ]; then |
Contributor
There was a problem hiding this comment.
P2: Stale worker-new-* containers can block future deployments because startup uses fixed worker-new-N names without pre-cleaning, and the abort guard exits before Phase 4 cleanup when all new workers fail to start. A failed deployment can leave worker-new-* containers that cause name collisions on the next run.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/worker-deploy.yml, line 462:
<comment>Stale worker-new-* containers can block future deployments because startup uses fixed worker-new-N names without pre-cleaning, and the abort guard exits before Phase 4 cleanup when all new workers fail to start. A failed deployment can leave worker-new-* containers that cause name collisions on the next run.</comment>
<file context>
@@ -0,0 +1,629 @@
+ # NOT touch the old workers. They are still serving jobs. Aborting here
+ # preserves the running system and avoids an outage.
+ # ─────────────────────────────────────────────────────────────────────────────
+ if [ "$WORKERS_STARTED" -eq 0 ]; then
+ log_message "ERROR" "❌ All new workers failed to start. Old workers left untouched. Deployment ABORTED."
+ log_message "ERROR" " Investigate new container logs before retrying."
</file context>
| const result = await ratelimit.limit(identifier); | ||
|
|
||
| if (!result.success) { | ||
| const resetTime = new Date(Date.now() + (result.reset || 0)); |
Contributor
There was a problem hiding this comment.
P2: result.reset appears to be an absolute timestamp (tests use Date.now()), but this code treats it as a duration by adding Date.now() again, inflating X-RateLimit-Reset and Retry-After.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/connector/src/plugins/rateLimit.ts, line 65:
<comment>`result.reset` appears to be an absolute timestamp (tests use `Date.now()`), but this code treats it as a duration by adding `Date.now()` again, inflating `X-RateLimit-Reset` and `Retry-After`.</comment>
<file context>
@@ -0,0 +1,251 @@
+ const result = await ratelimit.limit(identifier);
+
+ if (!result.success) {
+ const resetTime = new Date(Date.now() + (result.reset || 0));
+
+ reply.headers({
</file context>
|
|
||
| const data = await availabilityService.update( request.body as z.infer<typeof availabilityCreationBodySchema>, userId, Number.parseInt(availability)); | ||
|
|
||
| console.log("Data: ", data); |
Contributor
There was a problem hiding this comment.
P2: Custom agent: Avoid Logging Sensitive Information
Remove logging of the full availability update payload; it may include sensitive user data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/connector/src/routes/public/availability.route.ts, line 113:
<comment>Remove logging of the full availability update payload; it may include sensitive user data.</comment>
<file context>
@@ -0,0 +1,117 @@
+
+ const data = await availabilityService.update( request.body as z.infer<typeof availabilityCreationBodySchema>, userId, Number.parseInt(availability));
+
+ console.log("Data: ", data);
+
+ return ResponseFormatter.success(reply, data, 'Availability updated');
</file context>
…inalize onboarding on callback
Contributor
There was a problem hiding this comment.
3 issues found across 11 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/lib/getting-started/calendly/onboardingFinalization.ts">
<violation number="1" location="apps/web/lib/getting-started/calendly/onboardingFinalization.ts:98">
P2: Onboarding finalization is a non-atomic check-then-act; concurrent/retried calls can both pass the completedOnboarding guard and dispatch duplicate import jobs/create duplicate schedules before the user is updated.</violation>
</file>
<file name="apps/web/pages/settings/import/calendly.tsx">
<violation number="1" location="apps/web/pages/settings/import/calendly.tsx:9">
P2: getServerSideProps returns null for unauthenticated users, which is an invalid Next.js SSR return shape and will throw instead of returning props/redirect/notFound.</violation>
<violation number="2" location="apps/web/pages/settings/import/calendly.tsx:14">
P1: The new redirect passes userId in the callback URL, but the callback handler trusts req.query.userId without validating it against an authenticated session. That allows a caller with any valid OAuth code to supply another userId and link tokens to a different account. The callback should derive user identity from session (or validate state) instead of trusting the URL param.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| if (code && typeof code === "string") { | ||
| const query = new URLSearchParams({ | ||
| code, | ||
| userId: String(user.id), |
Contributor
There was a problem hiding this comment.
P1: The new redirect passes userId in the callback URL, but the callback handler trusts req.query.userId without validating it against an authenticated session. That allows a caller with any valid OAuth code to supply another userId and link tokens to a different account. The callback should derive user identity from session (or validate state) instead of trusting the URL param.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/pages/settings/import/calendly.tsx, line 14:
<comment>The new redirect passes userId in the callback URL, but the callback handler trusts req.query.userId without validating it against an authenticated session. That allows a caller with any valid OAuth code to supply another userId and link tokens to a different account. The callback should derive user identity from session (or validate state) instead of trusting the URL param.</comment>
<file context>
@@ -1,121 +1,34 @@
- await prisma.user.update({
- where: { id: user.id },
- data: { completedOnboarding: true },
+ userId: String(user.id),
});
- // Return the data as props
</file context>
| throw new Error("User not found"); | ||
| } | ||
|
|
||
| if (user.completedOnboarding) { |
Contributor
There was a problem hiding this comment.
P2: Onboarding finalization is a non-atomic check-then-act; concurrent/retried calls can both pass the completedOnboarding guard and dispatch duplicate import jobs/create duplicate schedules before the user is updated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/lib/getting-started/calendly/onboardingFinalization.ts, line 98:
<comment>Onboarding finalization is a non-atomic check-then-act; concurrent/retried calls can both pass the completedOnboarding guard and dispatch duplicate import jobs/create duplicate schedules before the user is updated.</comment>
<file context>
@@ -0,0 +1,161 @@
+ throw new Error("User not found");
+ }
+
+ if (user.completedOnboarding) {
+ return;
+ }
</file context>
| const { code, state } = context.query; | ||
| const session = await getServerSession(context); | ||
| const user = session?.user; | ||
| if (!user) return null; |
Contributor
There was a problem hiding this comment.
P2: getServerSideProps returns null for unauthenticated users, which is an invalid Next.js SSR return shape and will throw instead of returning props/redirect/notFound.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/pages/settings/import/calendly.tsx, line 9:
<comment>getServerSideProps returns null for unauthenticated users, which is an invalid Next.js SSR return shape and will throw instead of returning props/redirect/notFound.</comment>
<file context>
@@ -1,121 +1,34 @@
+ const { code, state } = context.query;
+ const session = await getServerSession(context);
+ const user = session?.user;
+ if (!user) return null;
if (code && typeof code === "string") {
</file context>
Ryukemeister
requested changes
Apr 1, 2026
Contributor
Ryukemeister
left a comment
Ryukemeister
left a comment
There was a problem hiding this comment.
hi there, thank you for your contribution. however I don't understand the purpose of the changes here in the first place. can you please pinpoint the main issues in the onboarding first? also I see lots and lots of file changes which dont seem to be necessary, can you revert those and make sure we only include the relevant changes, thank you!
…s already retrieved in ssr
…-impersonation flows
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
7 participants
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.
Description:
This PR improves the user experience by eliminating inconsistent and unexpected redirects between onboarding and dashboard flows.
Previously, users could experience abrupt page jumps or brief flashes due to overlapping client-side and server-side redirect logic. This made navigation feel unstable and confusing, especially when transitioning between onboarding and the main product.
What’s improved:
How it works:
User Impact:
``