packages/features/calAIPhone/AIPhoneServiceRegistry.ts (1)

78-85: Fix loss of type narrowing causing a potential TS error

Switching from this.defaultProvider to AIPhoneServiceRegistry.defaultProvider after the null-check breaks TypeScript’s control-flow narrowing, making the argument type string | null and likely causing a compile-time error.

Use this.defaultProvider here to retain the narrowing from the guard above.

Apply this diff:

-    return this.createProvider(AIPhoneServiceRegistry.defaultProvider, config);
+    return this.createProvider(this.defaultProvider, config);

If you want to keep class-qualified access, mirror the guard and use a non-null assertion:

if (!AIPhoneServiceRegistry.defaultProvider) {
  throw new Error("No default provider set. Please initialize the registry with a default provider or register at least one provider.");
}
return AIPhoneServiceRegistry.createProvider(AIPhoneServiceRegistry.defaultProvider!, config);

apps/web/app/api/auth/saml/userinfo/route.ts (3)

14-17: Harden Authorization header parsing: enforce Bearer scheme and handle malformed headers

Current logic accepts any scheme and may mis-parse tokens when extra spaces are present. It can also return an empty token. Enforce the Bearer scheme and extract robustly.

Apply this diff:

-  const parts = (authHeader || "").split(" ");
-  if (parts.length > 1) return parts[1];
+  if (authHeader) {
+    const match = authHeader.match(/^Bearer\s+(.+)$/i);
+    if (match && match[1].trim().length > 0) return match[1].trim();
+  }

---

32-34: Validate non-empty access_token at the schema level

Shift the empty-string check into zod. This removes the need for runtime length checks and yields clearer validation errors.

Apply this diff:

-const requestQuery = z.object({
-  access_token: z.string(),
-});
+const requestQuery = z.object({
+  access_token: z.string().min(1, "access_token must be a non-empty string"),
+});

---

41-48: Don’t downgrade authorization failures to 500; preserve/propagate HttpError status

Catching and throwing a generic Error converts all failures (including invalid/expired tokens) into 500s. Preserve HttpError when thrown upstream, and map unknown errors to an appropriate status.

Apply this diff:

-  } catch (error) {
-    const uid = uuid();
-    log.error(`trace: ${uid} Error getting user info from token: ${error}`);
-    throw new Error(`Error getting user info from token. trace: ${uid}`);
-  }
+  } catch (error: unknown) {
+    const uid = uuid();
+    log.error(`trace: ${uid} Error getting user info from token: ${String(error)}`);
+    if (error instanceof HttpError) {
+      throw error;
+    }
+    const statusCode =
+      typeof (error as any)?.statusCode === "number"
+        ? (error as any).statusCode
+        : typeof (error as any)?.status === "number"
+        ? (error as any).status
+        : 500;
+    throw new HttpError({
+      statusCode,
+      message:
+        statusCode === 401 || statusCode === 403
+          ? `Unauthorized. trace: ${uid}`
+          : `Error getting user info from token. trace: ${uid}`,
+    });
+  }

apps/api/v2/src/ee/event-types-private-links/controllers/event-types-private-links.controller.e2e-spec.ts (1)

114-123: Make the "list private links" test self-contained to avoid inter-test coupling

This test assumes at least one link exists (length >= 1) but doesn’t create one within the test, relying on earlier tests. That can lead to flaky tests if order changes or failures happen earlier.

Pre-create a link inside this test before listing:

 it("GET /v2/event-types/:eventTypeId/private-links - list private links", async () => {
+  // Ensure at least one link exists for this test in isolation
+  await request(app.getHttpServer())
+    .post(`/api/v2/event-types/${eventType.id}/private-links`)
+    .set("Authorization", `Bearer whatever`)
+    .send({ maxUsageCount: 1 })
+    .expect(201);
+
   const response = await request(app.getHttpServer())
     .get(`/api/v2/event-types/${eventType.id}/private-links`)
     .set("Authorization", `Bearer whatever`)
     .expect(200);

   expect(response.body.status).toBe(SUCCESS_STATUS);
   expect(Array.isArray(response.body.data)).toBe(true);
   expect(response.body.data.length).toBeGreaterThanOrEqual(1);
 });

apps/web/components/getting-started/steps-views/UserProfile.tsx (1)

60-66: Event type creation isn’t awaited; use mutateAsync so setup completes before redirect

createEventType.mutate is not Promise-based; wrapping it in Promise.all doesn’t await completion. This can cause racing with the redirect and incomplete default event creation. Switch to mutateAsync.

-          await Promise.all(
-            DEFAULT_EVENT_TYPES.map(async (event) => {
-              return createEventType.mutate(event);
-            })
-          );
+          await Promise.all(
+            DEFAULT_EVENT_TYPES.map((event) => createEventType.mutateAsync(event))
+          );

Optional: if partial failures are acceptable without blocking the flow, use Promise.allSettled and log/report the failures.

packages/features/calAIPhone/providers/retellAI/services/AgentService.ts (1)

58-65: Use HttpError consistently instead of generic Error for API-surface errors

updateAgent currently throws plain Error for validation and failure paths, while the rest of this service uses HttpError with status codes. Standardize to HttpError to propagate correct status codes to callers.

Apply:

-    if (!agentId?.trim()) {
-      throw new Error("Agent ID is required and cannot be empty");
-    }
+    if (!agentId?.trim()) {
+      throw new HttpError({ statusCode: 400, message: "Agent ID is required and cannot be empty" });
+    }
@@
-    if (!data || Object.keys(data).length === 0) {
-      throw new Error("Update data is required");
-    }
+    if (!data || Object.keys(data).length === 0) {
+      throw new HttpError({ statusCode: 400, message: "Update data is required" });
+    }
@@
-      throw new Error(`Failed to update agent ${agentId}`);
+      throw new HttpError({ statusCode: 500, message: `Failed to update agent ${agentId}` });

Also applies to: 75-76

packages/features/calAIPhone/providers/adapters/PrismaPhoneNumberRepositoryAdapter.ts (1)

41-56: Adapter drops outboundAgentId on create — repository doesn't accept it; fix required

Short: PrismaPhoneNumberRepository.createPhoneNumber does not accept or persist outboundAgentId, while the adapter/interface expect it and PrismaTransactionAdapter already supports it. This will silently drop outboundAgentId for non-transactional creates.

Files to change:

• packages/lib/server/repository/PrismaPhoneNumberRepository.ts — add outboundAgentId to createPhoneNumber signature, include in data and select.
• packages/features/calAIPhone/providers/adapters/PrismaPhoneNumberRepositoryAdapter.ts — pass outboundAgentId through to the repository.
• (Verify callers that call the repository directly and update if needed.)

Proposed diffs:

packages/lib/server/repository/PrismaPhoneNumberRepository.ts

   static async createPhoneNumber({
     phoneNumber,
     provider,
     userId,
-    teamId,
+    teamId,
+    outboundAgentId,
   }: {
     phoneNumber: string;
     provider?: string;
     userId: number;
-    teamId?: number;
+    teamId?: number;
+    outboundAgentId?: string | null;
   }) {
     return await prisma.calAiPhoneNumber.create({
       select: {
         id: true,
         phoneNumber: true,
         provider: true,
         userId: true,
         teamId: true,
+        inboundAgentId: true,
+        outboundAgentId: true,
         subscriptionStatus: true,
         createdAt: true,
         updatedAt: true,
       },
       data: {
         provider,
         userId,
         teamId,
+        outboundAgentId,
         phoneNumber,
       },
     });
   }

packages/features/calAIPhone/providers/adapters/PrismaPhoneNumberRepositoryAdapter.ts

   async createPhoneNumber(params: {
     phoneNumber: string;
     provider?: string;
     userId: number;
     teamId?: number;
     outboundAgentId?: string | null;
   }): Promise<PhoneNumberData> {
     const createParams = {
       phoneNumber: params.phoneNumber,
       provider: params.provider,
       userId: params.userId,
       teamId: params.teamId,
+      outboundAgentId: params.outboundAgentId ?? undefined,
     };

     return await PrismaPhoneNumberRepository.createPhoneNumber(createParams);
   }

Reason: keep interface and transaction behavior consistent; prefer adding the field to the repository so callers that aren't using transactions still persist outboundAgentId. If this is intentional (field must not be set outside transactions), remove the parameter from the interface and adapter instead.

apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts (1)

170-173: Avoid disabling ESLint; omit property without introducing an unused binding

You can drop the rule suppression and keep the same runtime behavior by copying and deleting the property.

-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    const { videoCallUrl, ...userDefinedMetadata } = databaseMetadata;
-
-    return userDefinedMetadata;
+    const userDefinedMetadata = { ...databaseMetadata };
+    delete userDefinedMetadata.videoCallUrl;
+    return userDefinedMetadata;

packages/features/calAIPhone/AIPhoneServiceRegistry.ts (2)

94-96: Prefer consistent static access style within the class

Elsewhere in this class, static fields are accessed via this. Stick to one convention for readability; using this also plays better with control-flow narrowing.

Apply this diff:

-    return AIPhoneServiceRegistry.defaultProvider;
+    return this.defaultProvider;

If you intend to standardize on class-qualified access, consider updating all other static references in the class for consistency.

---

115-117: Minor consistency nit: align static access style

Same note as above—prefer one style of static access within the class.

Apply this diff:

-    return AIPhoneServiceRegistry.initialized;
+    return this.initialized;

apps/web/app/api/auth/saml/userinfo/route.ts (2)

25-29: Simplify query param handling; avoid unnecessary temporary array

The current array dance adds complexity without benefit. Just return the token if non-empty; otherwise fall through to the 401.

Apply this diff:

-  const access_token = tokenParse.data.access_token;
-  arr = arr.concat(access_token);
-  if (arr[0].length > 0) return arr[0];
+  const tokenFromQuery = tokenParse.data.access_token;
+  if (tokenFromQuery.length > 0) return tokenFromQuery;

---

21-23: Use warn-level for expected validation failures

Missing/invalid tokens are common and expected; logging them as errors can create noise. Suggest downgrading to warn.

Option:

-    log.error(`Error parsing request query: ${tokenParse.error} trace ${uid}`);
+    log.warn(`Invalid request query for token extraction: ${tokenParse.error} trace ${uid}`);

apps/api/v2/src/modules/cal-unified-calendars/pipes/google-calendar-event-input.pipe.spec.ts (3)

14-16: Redundant beforeAll — re-instantiation already happens in beforeEach.

beforeAll is unnecessary since pipe is recreated before each test. Removing it reduces noise.

-  beforeAll(() => {
-    pipe = new GoogleCalendarEventInputPipe();
-  });

---

112-174: Accessing private helpers directly makes tests brittle. Consider testing via public API.

Using pipe["transformResponseStatus"] and pipe["transformEventStatus"] ties tests to internal details. Prefer asserting these mappings through the public transform method, or extract the mapping functions and export them for direct testing.

---

22-80: Missing coverage for attendees transformation.

Given recent touch-ups in attendees mapping in the pipe, add at least one test that exercises transforming attendees from UpdateUnifiedCalendarEventInput to the expected Google payload.

Example to add near existing transform tests:

     it("should handle null description", () => {
       const input: UpdateUnifiedCalendarEventInput = {
         description: null,
       };

       const expectedOutput = {
         description: null,
       };

       const result = pipe.transform(input);
       expect(result).toEqual(expectedOutput);
     });
+
+    it("should transform attendees", () => {
+      const input: UpdateUnifiedCalendarEventInput = {
+        attendees: [
+          { email: "a@example.com", name: "Alice", responseStatus: CalendarEventResponseStatus.ACCEPTED },
+          { email: "b@example.com", name: "Bob", responseStatus: CalendarEventResponseStatus.PENDING },
+        ],
+      };
+      const result = pipe.transform(input);
+      // Be flexible on exact shape if needed; minimally assert mapped emails and response statuses.
+      expect(result.attendees).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({ email: "a@example.com" }),
+          expect.objectContaining({ email: "b@example.com" }),
+        ])
+      );
+    });

apps/api/v2/src/ee/event-types-private-links/controllers/event-types-private-links.controller.e2e-spec.ts (2)

15-16: Organization/OAuth scaffolding seems unused; streamline test or make intent explicit

The created organization and OAuth client aren’t used by the test cases. This adds setup time and potential cleanup burden.

• Option A (preferred): Remove org/OAuth creation entirely (and drop TeamRepositoryFixture/OAuthClientRepositoryFixture usage and related helper). This will speed up the test and reduce side effects.
• Option B: If you intentionally keep it for side-effects, explicitly acknowledge the ignored result to satisfy linters/readers.

Minimal change for Option B:

-    await createOAuthClient(organization.id);
+    void (await createOAuthClient(organization.id));

If you choose Option A (bigger edit), I can help produce a patch that:

• Removes the organization and OAuth client creation and their imports/fixtures
• Simplifies beforeAll to only create the user and event type
• Removes the createOAuthClient helper

Also applies to: 62-62

---

164-173: Clean up all created entities to prevent test DB pollution

Only the event type is deleted. Consider also deleting:

• OAuth client created for the organization
• The organization itself
• The user

If repository fixtures don’t expose delete helpers, I can help add them to keep tests hermetic and faster to iterate.

apps/web/components/getting-started/steps-views/UserProfile.tsx (2)

121-129: Remove unused placeholder on hidden input and fix likely typo in Tailwind class

• The placeholder on a hidden input has no effect and adds an unlocalized string in TSX.
• Likely typo: focus:ring-empthasis should be focus:ring-emphasis.

         <input
           ref={avatarRef}
           type="hidden"
           name="avatar"
           id="avatar"
-          placeholder="URL"
-          className="border-default focus:ring-empthasis mt-1 block w-full rounded-sm border px-3 py-2 text-sm focus:border-gray-800 focus:outline-none"
+          className="border-default focus:ring-emphasis mt-1 block w-full rounded-sm border px-3 py-2 text-sm focus:border-gray-800 focus:outline-none"
           defaultValue={imageSrc}
         />

---

92-96: Drop unnecessary async from updateProfileHandler

No await is used; returning a Promise here is unnecessary.

-  async function updateProfileHandler(newAvatar: string) {
-    avatarMutation.mutate({
-      avatarUrl: newAvatar,
-    });
-  }
+  function updateProfileHandler(newAvatar: string) {
+    avatarMutation.mutate({
+      avatarUrl: newAvatar,
+    });
+  }

packages/features/calAIPhone/providers/retellAI/services/AgentService.ts (1)

200-221: Nit: Add a short comment clarifying that name is intentionally not persisted here

Per prior maintainers’ decision (captured in team learnings), name is not persisted by updateAgentConfiguration. To avoid future churn, add a brief inline comment.

Apply:

   async updateAgentConfiguration({
     id,
     userId,
     name,
     generalPrompt,
     beginMessage,
     generalTools,
     voiceId,
     updateLLMConfiguration,
   }: {
@@
-  }) {
+  }) {
+    // Intentionally not persisting `name` here as per current product decision.
+    // If requirements change, persist via repository in this method.
     const agent = await this.agentRepository.findByIdWithAdminAccess({
       id,
       userId,
     });

Note: This acknowledges existing behavior based on prior decisions.

packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts (1)

173-174: Remove redundant await in simple return statements

These methods simply proxy to the service and don’t need await. Dropping it avoids wrapping/rethrowing and a tiny bit of overhead.

Apply:

-    return await this.service.generatePhoneNumberCheckoutSession(params);
+    return this.service.generatePhoneNumberCheckoutSession(params);
@@
-    return await this.service.cancelPhoneNumberSubscription(params);
+    return this.service.cancelPhoneNumberSubscription(params);
@@
-    return await this.service.updatePhoneNumberWithAgents(params);
+    return this.service.updatePhoneNumberWithAgents(params);
@@
-    return await this.service.listAgents(params);
+    return this.service.listAgents(params);
@@
-    return await this.service.getAgentWithDetails(params);
+    return this.service.getAgentWithDetails(params);
@@
-    return await this.service.updateAgentConfiguration(params);
+    return this.service.updateAgentConfiguration(params);
@@
-    return await this.service.deleteAgent(params);
+    return this.service.deleteAgent(params);
@@
-    return await this.service.createTestCall(params);
+    return this.service.createTestCall(params);

Also applies to: 181-182, 191-192, 202-203, 210-211, 246-247, 249-251, 263-264

packages/features/calAIPhone/providers/interfaces/PhoneNumberRepositoryInterface.ts (1)

68-73: Strengthen typing and harmonize naming for clarity

• subscriptionStatus is typed as string | null in PhoneNumberData while the interface already imports PhoneNumberSubscriptionStatus. Prefer the enum type for stronger guarantees.
• Naming is inconsistent: updateAgents uses inboundProviderAgentId/outboundProviderAgentId while PhoneNumberData uses inboundAgentId/outboundAgentId. Consider aligning naming across interface/data to reduce confusion.

Apply for subscriptionStatus:

 export interface PhoneNumberData {
   id: number;
   phoneNumber: string;
   userId: number | null;
   teamId: number | null;
-  subscriptionStatus: string | null;
+  subscriptionStatus: PhoneNumberSubscriptionStatus | null;
   stripeSubscriptionId: string | null;
   stripeCustomerId: string | null;
   provider: string | null;
   inboundAgentId: string | null;
   outboundAgentId: string | null;
   createdAt: Date;
   updatedAt: Date;
 }

For naming consistency, consider:

• Either rename PhoneNumberData fields to inboundProviderAgentId/outboundProviderAgentId
• Or rename updateAgents params to inboundAgentId/outboundAgentId

If you want, I can generate a follow-up PR to unify these names across provider, adapter, and repository.

Also applies to: 79-91

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).

Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.

Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.