Conversation
…ons (#57) Upgrade typescript dependency to 6.0.3 across all monorepo workspaces and root package.json. Add ignoreDeprecations 6.0 compilerOption to tsconfig base and update package-level tsconfigs with rootDir and modern lib targets. Patch zod-prisma-types to use fs.rmSync avoiding deprecated fs.rmdirSync. Re-export PLATFORM_PERMISSION from constants avoiding circular enum/types issue. Update vitest.config.mts and tsconfig exclusions to isolate test files from production typechecking. Verify turbo type-check, Vitest unit tests, and Next.js Turbopack production build.
…her and Teams management
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_47a6b250-9296-49a6-87d3-b65358548b29) |
There was a problem hiding this comment.
Code Review
This pull request upgrades TypeScript to version 6.0.3 across the monorepo, adds a TypeScript 7.0 migration roadmap, applies a patch to zod-prisma-types, and introduces new Playwright E2E tests for the Crove App Switcher and multi-tenant security. Feedback on the new E2E tests highlights critical issues where conditional blocks and weak assertions could allow tests to pass silently, potentially masking authentication bypasses or rendering failures.
| test("tRPC endpoint /api/trpc/viewer.teams.list should require authentication", async ({ request }) => { | ||
| const response = await request.get("/api/trpc/viewer/teams.list?batch=1&input=%7B%7D"); | ||
|
|
||
| // When unauthenticated, tRPC should return UNAUTHORIZED (401) or error in batch response | ||
| expect([200, 401, 400]).toContain(response.status()); | ||
| if (response.status() === 200) { | ||
| const json = await response.json(); | ||
| // In batch tRPC, error is inside payload array | ||
| if (Array.isArray(json) && json[0]?.error) { | ||
| expect(json[0].error.json.message).toBeDefined(); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
This test is designed to verify that the /api/trpc/viewer.teams.list endpoint requires authentication. However, if the endpoint is insecurely exposed and returns a 200 OK status with actual data, the test will silently pass because the assertion expect(json[0].error.json.message).toBeDefined() is wrapped inside a conditional if (Array.isArray(json) && json[0]?.error) block. If there is no error, the block is skipped and the test passes. We must explicitly assert that the response contains an error when a 200 status is returned.
test("tRPC endpoint /api/trpc/viewer.teams.list should require authentication", async ({ request }) => {
const response = await request.get("/api/trpc/viewer/teams.list?batch=1&input=%7B%7D");
// When unauthenticated, tRPC should return UNAUTHORIZED (401) or error in batch response
expect([200, 401, 400]).toContain(response.status());
if (response.status() === 200) {
const json = await response.json();
expect(Array.isArray(json)).toBe(true);
expect(json[0]).toHaveProperty("error");
expect(json[0].error.json.message).toBeDefined();
}
});| test("tRPC endpoint /api/trpc/viewer.organizations.listCurrent should require authentication", async ({ | ||
| request, | ||
| }) => { | ||
| const response = await request.get("/api/trpc/viewer/organizations.listCurrent?batch=1&input=%7B%7D"); | ||
|
|
||
| expect([200, 401, 400]).toContain(response.status()); | ||
| }); |
There was a problem hiding this comment.
This test only asserts that the response status is one of [200, 401, 400]. If the endpoint is insecurely exposed and returns a 200 OK with the actual organization list, the test will pass without verifying if an error was returned. We must assert that if a 200 status is returned, the body contains a tRPC error indicating that authentication is required.
test("tRPC endpoint /api/trpc/viewer.organizations.listCurrent should require authentication", async ({
request,
}) => {
const response = await request.get("/api/trpc/viewer/organizations.listCurrent?batch=1&input=%7B%7D");
expect([200, 401, 400]).toContain(response.status());
if (response.status() === 200) {
const json = await response.json();
expect(Array.isArray(json)).toBe(true);
expect(json[0]).toHaveProperty("error");
}
});| const appSwitcherTrigger = page.locator('button[title="Crove Ecosystem Apps"]'); | ||
| const triggerCount = await appSwitcherTrigger.count(); | ||
| expect(triggerCount).toBeGreaterThanOrEqual(0); | ||
|
|
||
| if (triggerCount > 0) { | ||
| await appSwitcherTrigger.first().click(); | ||
|
|
||
| // Check Crove Suite section | ||
| const croveSuiteSection = page.getByText("Crove Suite"); | ||
| await expect(croveSuiteSection).toBeVisible(); | ||
|
|
||
| // Check key applications listed in the switcher | ||
| await expect(page.getByText("Crove Cal")).toBeVisible(); | ||
| await expect(page.getByText("Crove Post")).toBeVisible(); | ||
| await expect(page.getByText("Crove Sign")).toBeVisible(); | ||
| await expect(page.getByText("Crove CRM")).toBeVisible(); | ||
|
|
||
| // Check DOS Ecosystem section | ||
| const dosSection = page.getByText("DOS Ecosystem"); | ||
| await expect(dosSection).toBeVisible(); | ||
| await expect(page.getByText("DOS ID")).toBeVisible(); | ||
| await expect(page.getByText("DOS.Me")).toBeVisible(); | ||
| await expect(page.getByText("DOS AI")).toBeVisible(); | ||
| } |
There was a problem hiding this comment.
The test currently uses a conditional check if (triggerCount > 0) to decide whether to click the App Switcher and assert its contents. This is a testing anti-pattern because if the App Switcher trigger fails to render (count is 0), the test will silently pass without executing any of the core assertions. E2E tests should be deterministic; if the App Switcher is expected to be present, we should explicitly assert its visibility and fail the test if it is missing.
const appSwitcherTrigger = page.locator('button[title="Crove Ecosystem Apps"]');
await expect(appSwitcherTrigger).toBeVisible();
await appSwitcherTrigger.click();
// Check Crove Suite section
const croveSuiteSection = page.getByText("Crove Suite");
await expect(croveSuiteSection).toBeVisible();
// Check key applications listed in the switcher
await expect(page.getByText("Crove Cal")).toBeVisible();
await expect(page.getByText("Crove Post")).toBeVisible();
await expect(page.getByText("Crove Sign")).toBeVisible();
await expect(page.getByText("Crove CRM")).toBeVisible();
// Check DOS Ecosystem section
const dosSection = page.getByText("DOS Ecosystem");
await expect(dosSection).toBeVisible();
await expect(page.getByText("DOS ID")).toBeVisible();
await expect(page.getByText("DOS.Me")).toBeVisible();
await expect(page.getByText("DOS AI")).toBeVisible();| const appSwitcherTrigger = page.locator('button[title="Crove Ecosystem Apps"]'); | ||
|
|
||
| if ((await appSwitcherTrigger.count()) > 0) { | ||
| await appSwitcherTrigger.first().click(); | ||
|
|
||
| // Check anchor tags | ||
| const postLink = page.locator('a[href="https://post.crove.com"]'); | ||
| await expect(postLink).toBeAttached(); | ||
|
|
||
| const signLink = page.locator('a[href="https://sign.crove.com"]'); | ||
| await expect(signLink).toBeAttached(); | ||
|
|
||
| const dosMeLink = page.locator('a[href="https://dos.me"]'); | ||
| await expect(dosMeLink).toBeAttached(); | ||
| } |
There was a problem hiding this comment.
Similar to the previous test, wrapping the assertions in a conditional if ((await appSwitcherTrigger.count()) > 0) block allows the test to pass silently if the trigger button is missing. We should explicitly assert that the trigger is visible and click it to ensure the URL assertions are always executed.
const appSwitcherTrigger = page.locator('button[title="Crove Ecosystem Apps"]');
await expect(appSwitcherTrigger).toBeVisible();
await appSwitcherTrigger.click();
// Check anchor tags
const postLink = page.locator('a[href="https://post.crove.com"]');
await expect(postLink).toBeAttached();
const signLink = page.locator('a[href="https://sign.crove.com"]');
await expect(signLink).toBeAttached();
const dosMeLink = page.locator('a[href="https://dos.me"]');
await expect(dosMeLink).toBeAttached();Add /api/health endpoint verifying database connectivity, latency and uptime. Update .env.example with CROVE_BREVO_API_KEY.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4b4eb55d-d3a0-4228-a178-f93c2bdbe495) |
…aration Upgrade target to ES2022 in nextjs.json, react-library.json, platform packages, lib, and testing packages. Verify 114/114 packages pass type-check.
Summary
Test plan
Note
Medium Risk
Wide TypeScript and Prisma generator changes can surface latent type or codegen issues; the health endpoint exposes DB status to unauthenticated callers (standard for probes but worth load-balancer exposure review).
Overview
This PR promotes dev → main with a monorepo-wide TypeScript 5.9.3 → 6.0.3 bump, adds
ignoreDeprecations: "6.0"on shared tsconfigs, and tightens compiler settings (rootDir, ES2022 targets on embed packages, excluding**/*.test.*from apptscruns) so type-check stays green across workspaces.Prisma codegen moves to
zod-prisma-types@3.3.11with a Yarn patch that prefersfs.rmSyncover deprecatedrmdirSync.PLATFORM_PERMISSIONis defined in@calcom/platform-constantsand re-exported from@calcom/platform-typesto simplify platform package typing.Operational / product additions: a new
GET/HEAD/api/healthroute reports DB connectivity (200 vs 503) with Vitest coverage;.env.exampledocumentsCROVE_BREVO_API_KEYfor existing Brevo CRM integration; Playwright specs cover the Crove app switcher, unauthenticated teams/org redirects, and auth expectations on teams/organizations tRPC;docs/TypeScript-7-Migration-Roadmap.mdoutlines the next compiler upgrade. Turbopost-installcaching and Vitest.nextexclusion are adjusted for the new toolchain.Reviewed by Cursor Bugbot for commit 4f814f5. Configure here.