Skip to content

chore: Return 200 if bot is hitting /router#23727

Closed
joeauyeung wants to merge 16 commits intomainfrom
return-if-bot
Closed

chore: Return 200 if bot is hitting /router#23727
joeauyeung wants to merge 16 commits intomainfrom
return-if-bot

Conversation

@joeauyeung
Copy link
Contributor

What does this PR do?

This PR early returns if a bot/crawler is hitting /router endpoint.

@joeauyeung joeauyeung requested a review from a team as a code owner September 9, 2025 21:34
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 9, 2025

Walkthrough

Adds BotID integration and resilient edge-config reads: adds the botid dependency, introduces VERCEL\_BOTID\_ENABLED in .env.example and turbo.json, initializes BotID client protections in apps/web/instrumentation-client.ts (gated by NEXT\_PUBLIC\_VERCEL\_BOTID\_ENABLED), conditionally injects the BotID Next.js plugin in apps/web/next.config.js, performs an early server-side checkBotId in apps/web/server/lib/router/getServerSideProps.ts to return an access-denied props payload for verified bots, and adds an internal safeGet<T> wrapper in apps/web/middleware.ts for guarded edge-config reads.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title concisely and clearly conveys the primary change—adding an HTTP 200 early return for bot requests to the /router endpoint—without extraneous details or jargon, making it understandable at a glance.
Description Check ✅ Passed The description directly describes the key behavior change—early returning for bot or crawler requests on the /router endpoint—which matches the modifications made in the server-side code.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch return-if-bot

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@graphite-app graphite-app bot requested a review from a team September 9, 2025 21:34
@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Sep 9, 2025
@dosubot dosubot bot added the foundation label Sep 9, 2025
@vercel
Copy link

vercel bot commented Sep 9, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 10, 2025 5:16am
cal-eu Ignored Ignored Sep 10, 2025 5:16am

}
} catch (error) {
logger.error("Error checking if user agent is a bot in web middlware:", error);
return NextResponse.next();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is any error then we should fail silently to the user

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
apps/web/middleware.ts (1)

106-108: Log level for missing UA

We already fail open; downgrade to warn/debug to reduce noise for benign cases (privacy tools strip UA).

-      logger.error("Missing user agent in web middleware");
+      logger.warn("Missing user agent in web middleware");
🧹 Nitpick comments (2)
apps/web/middleware.ts (2)

116-116: Typo in log message

“middlware” → “middleware”.

-      logger.error("Error checking if user agent is a bot in web middlware:", error);
+      logger.error("Error checking if user agent is a bot in web middleware:", error);

102-119: Micro perf: move bot check earlier

To minimize overhead for bots, consider checking /router immediately after static-file checks, before expensive header enrichment and rewrites.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a2d0cbf and 3f199a7.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (2)
  • apps/web/middleware.ts (2 hunks)
  • apps/web/package.json (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/web/middleware.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/web/middleware.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/web/middleware.ts
🧬 Code graph analysis (1)
apps/web/middleware.ts (1)
apps/api/v2/src/lib/logger.bridge.ts (1)
  • error (77-79)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (4)
apps/web/package.json (1)

96-96: Confirm isbot ESM import and Edge runtime compatibility

  • ESM import for v5.x: import { isbot } from "isbot";
  • isbot@^5.1.30 is pure JS, ships as ESM, and runs safely in Edge/Worker runtimes (regex fallback only on engines without lookbehind).
    Consider pinning to 5.1.30 if upstream volatility is a concern.
apps/web/middleware.ts (3)

7-7: Confirm logger is Edge-safe

Ensure @calcom/lib/logger has no Node-only APIs so it runs in middleware (Edge runtime).


12-19: Silent edge-config failures

LGTM. Swallowing edge-config errors and continuing aligns with resilience goals.


2-2: Fix import: isbot should be a default import (not named)

isbot v5 exports a default function; { isbot } will be undefined at runtime.

Apply:

-import { isbot } from "isbot";
+import isbot from "isbot";

Likely an incorrect or invalid review comment.

Comment on lines 102 to 119
if (url.pathname.startsWith("/router")) {
const userAgent = req.headers.get("user-agent");

if (!userAgent) {
logger.error("Missing user agent in web middleware");
return NextResponse.next();
}

try {
const isBot = isbot(userAgent);
if (isBot) {
return NextResponse.status(200);
}
} catch (error) {
logger.error("Error checking if user agent is a bot in web middlware:", error);
return NextResponse.next();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Router bot short-circuit won’t run: add /router to middleware matcher

This branch never executes because /router isn’t listed in config.matcher (see Lines 197-208). Add an entry so the middleware actually intercepts these requests.

Example (choose exact or wildcard based on your routing):

export const config = {
  matcher: [
    // ...existing
    "/router", // exact
    // or: "/router/:path*", if you expect subpaths
  ],
};
🤖 Prompt for AI Agents
In apps/web/middleware.ts around lines 102 to 119, the router bot short-circuit
branch never runs because "/router" (or its wildcard) is not included in the
middleware config.matcher defined later (lines ~197-208); update the export
const config.matcher array to include either "/router" for exact matching or
"/router/:path*" (or equivalent wildcard) if you expect subpaths so the
middleware intercepts those requests and the bot-check branch can execute.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2025

E2E results are ready!

@github-actions github-actions bot added the ❗️ .env changes contains changes to env variables label Sep 9, 2025
Comment on lines +54 to +57
{
path: "/router",
method: "POST",
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting this message so thought I would just include these in the config

Possible misconfiguration of Vercel BotId. Ensure that the client-side protection is properly configured for 'POST <your protected endpoint>'. Add the following item to your BotId client side protection: { path: '<your protected endpoint>', method: 'POST', }

informAboutDuplicateTranslations();
const plugins = [];

if (process.env.VERCEL_BOTID_ENABLED === true) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only add the plug in if enabled via env variable

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/middleware.ts (1)

10-17: safeGet doesn’t catch async rejections; middleware may still crash

get() returns a Promise; returning it inside try/catch won’t catch rejections. Await it to swallow errors per intent.

-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-const safeGet = async <T = any>(key: string): Promise<T | undefined> => {
-  try {
-    return get<T>(key);
-  } catch (error) {
-    // Don't crash if EDGE_CONFIG env var is missing
-  }
-};
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+const safeGet = async <T = any>(key: string): Promise<T | undefined> => {
+  try {
+    return await get<T>(key);
+  } catch {
+    // Don't crash if EDGE_CONFIG env var is missing
+    return undefined;
+  }
+};
♻️ Duplicate comments (1)
apps/web/middleware.ts (1)

171-188: If middleware should handle /router, add it to matcher (else this path never hits middleware)

Only needed if you still plan a middleware short‑circuit for bots; otherwise ignore. This mirrors the earlier note.

   matcher: [
     // Routes to enforce CSP
     "/auth/login",
     "/login",
+    // Router endpoints (enable if handling bots here)
+    "/router",
+    "/router/:path*",
     // Routes to set cookies
     "/apps/installed",
     "/auth/logout",
🧹 Nitpick comments (1)
.env.example (1)

476-477: Clarify expected values for VERCEL_BOTID_ENABLED

Env vars are strings; downstream checks compare booleans incorrectly. Document accepted values to avoid misconfiguration.

-VERCEL_BOTID_ENABLED=
+# Enable BotID protections (reads as a string). Accepts "true" or "1".
+VERCEL_BOTID_ENABLED=
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3f199a7 and 382cacb.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (7)
  • .env.example (1 hunks)
  • apps/web/instrumentation-client.ts (2 hunks)
  • apps/web/middleware.ts (1 hunks)
  • apps/web/next.config.js (2 hunks)
  • apps/web/package.json (1 hunks)
  • apps/web/server/lib/router/getServerSideProps.ts (2 hunks)
  • turbo.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/package.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/web/next.config.js
  • apps/web/server/lib/router/getServerSideProps.ts
  • apps/web/middleware.ts
  • apps/web/instrumentation-client.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/web/server/lib/router/getServerSideProps.ts
  • apps/web/middleware.ts
  • apps/web/instrumentation-client.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/web/server/lib/router/getServerSideProps.ts
  • apps/web/middleware.ts
  • apps/web/instrumentation-client.ts
🪛 Gitleaks (8.27.2)
.env.example

[high] 474-476: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🔇 Additional comments (5)
turbo.json (1)

199-199: LGTM: propagate VERCEL_BOTID_ENABLED to tasks

This ensures the flag is available at build/runtime for Next config and client bundling.

apps/web/next.config.js (2)

1-1: LGTM: BotID plugin import

Import placement looks fine.


114-116: Use string-based check for VERCEL_BOTID_ENABLED
Node.js process.env values are always strings, so process.env.VERCEL_BOTID_ENABLED === true never matches (nodejs.org)
Update to:

const botIdEnabled = ["1","true"].includes(String(process.env.VERCEL_BOTID_ENABLED).toLowerCase());
if (botIdEnabled) {
  plugins.push(withBotId);
}

Verify if withBotId requires invocation (withBotId() vs withBotId).

apps/web/instrumentation-client.ts (1)

5-5: LGTM: client BotID init import

No concerns.

apps/web/server/lib/router/getServerSideProps.ts (1)

2-2: Call checkBotId without arguments — according to Vercel BotID docs, in server code (API routes, getServerSideProps, Server Actions) you simply use await checkBotId() with no req/res/context parameters (vercel.com)

Comment on lines 47 to 68
if (process.env.VERCEL_BOTID_ENABLED === true) {
initBotId({
protect: [
{
path: "/router",
method: "GET",
},
{
path: "/router",
method: "POST",
},
{
path: "/router/embed",
method: "GET",
},
{
path: "/router/embed",
method: "POST",
},
],
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Client BotID gating never triggers (string vs boolean); fix and keep consistent with Next config

As written, initBotId won’t run.

-if (process.env.VERCEL_BOTID_ENABLED === true) {
+const botIdEnabled = ["1", "true"].includes(String(process.env.VERCEL_BOTID_ENABLED).toLowerCase());
+if (botIdEnabled) {
   initBotId({
     protect: [
       { path: "/router", method: "GET" },
       { path: "/router", method: "POST" },
       { path: "/router/embed", method: "GET" },
       { path: "/router/embed", method: "POST" },
     ],
   });
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (process.env.VERCEL_BOTID_ENABLED === true) {
initBotId({
protect: [
{
path: "/router",
method: "GET",
},
{
path: "/router",
method: "POST",
},
{
path: "/router/embed",
method: "GET",
},
{
path: "/router/embed",
method: "POST",
},
],
});
}
// apps/web/instrumentation-client.ts
// …
// Normalize the VERCEL_BOTID_ENABLED env var (which is always a string)
// and treat "1" or "true" (case-insensitive) as enabled.
const botIdEnabled = ["1", "true"].includes(
String(process.env.VERCEL_BOTID_ENABLED).toLowerCase(),
);
if (botIdEnabled) {
initBotId({
protect: [
{ path: "/router", method: "GET" },
{ path: "/router", method: "POST" },
{ path: "/router/embed", method: "GET" },
{ path: "/router/embed", method: "POST" },
],
});
}
// …
🤖 Prompt for AI Agents
In apps/web/instrumentation-client.ts around lines 47 to 68, the check uses
strict boolean comparison against process.env.VERCEL_BOTID_ENABLED which is a
string in Node envs, so initBotId never runs; change the condition to check the
string value (e.g., process.env.VERCEL_BOTID_ENABLED === "true" or normalize
with toLowerCase() === "true") or coerce appropriately, and ensure the
environment variable is set as the string "true" in your Next/VERCEL config to
keep behavior consistent with the server config.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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)
apps/web/server/lib/router/getServerSideProps.ts (2)

12-13: Use resolvedUrl for more robust embed detection.

context.resolvedUrl is generally safer than req.url (rewrites, basePath, etc.).

-  const isEmbed = hasEmbedPath(context.req.url || "");
+  const isEmbed = hasEmbedPath(context.resolvedUrl || context.req.url || "");

17-25: Add X-Robots-Tag to avoid indexing bot-denied responses.

Explicitly send 200 and discourage indexing.

-      return {
+      context.res.statusCode = 200;
+      context.res.setHeader("X-Robots-Tag", "noindex, nofollow");
+      return {
         props: {
           isEmbed,
           message: null,
           form: null,
           errorMessage: "Access denied",
         },
       };
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ea7f7c8 and ce75065.

📒 Files selected for processing (1)
  • apps/web/server/lib/router/getServerSideProps.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/web/server/lib/router/getServerSideProps.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/web/server/lib/router/getServerSideProps.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/web/server/lib/router/getServerSideProps.ts
🧬 Code graph analysis (1)
apps/web/server/lib/router/getServerSideProps.ts (1)
packages/features/routing-forms/lib/getRoutedUrl.ts (1)
  • hasEmbedPath (49-52)
🔇 Additional comments (3)
apps/web/server/lib/router/getServerSideProps.ts (3)

2-2: Import looks good for server-only usage.

No concerns with bringing checkBotId into this SSR handler.


17-25: Return-shape alignment with 429 branch — nice.

The props structure matches the 429 path; avoids consumer breakage.


15-16: Harden bot check: fail-open on BotID errors
Wrap checkBotId() (imported from botid/server) in a try/catch and default isBot to false on errors to prevent 5xx failures—checkBotId() requires no req/res or manual headers and returns { isBot: boolean }.

-    const verification = await checkBotId();
-    if (verification.isBot) {
+    let isBot = false;
+    try {
+      const verification = await checkBotId();
+      isBot = verification.isBot;
+    } catch {
+      isBot = false; // fail-open on error
+    }
+    if (isBot) {

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
apps/web/server/lib/router/getServerSideProps.ts (1)

14-26: Fix env gating: current check only runs in E2E and treats "false" as truthy

The condition is inverted and relies on truthiness of strings. This will skip bot checks in prod and may unexpectedly enable them when env vars are set to "false".

Apply:

-  if (process.env.NEXT_PUBLIC_IS_E2E && process.env.VERCEL_BOTID_ENABLED) {
-    const verification = await checkBotId();
-    if (verification.isBot) {
-      return {
-        props: {
-          isEmbed,
-          message: null,
-          form: null,
-          errorMessage: "Access denied",
-        },
-      };
-    }
-  }
+  const isE2E = process.env.NEXT_PUBLIC_IS_E2E === "true";
+  const botIdEnabled = process.env.VERCEL_BOTID_ENABLED === "true";
+  if (!isE2E && botIdEnabled) {
+    try {
+      const verification = await checkBotId({ req: context.req });
+      if (verification.isBot) {
+        return {
+          props: {
+            isEmbed,
+            message: null,
+            form: null,
+            errorMessage: "Access denied",
+          },
+        };
+      }
+    } catch {
+      // Fail open: keep /router functional if BotID check errors.
+    }
+  }
🧹 Nitpick comments (2)
apps/web/server/lib/router/getServerSideProps.ts (2)

17-24: Be explicit: set HTTP 200 for bot branch

Next defaults to 200, but setting it avoids ambiguity and future regressions.

-    if (verification.isBot) {
+    if (verification.isBot) {
+      context.res.statusCode = 200;
       return {

12-12: Prefer resolvedUrl over req.url for embed detection

resolvedUrl is stable across rewrites; req.url can vary under proxies/rewrites.

-  const isEmbed = hasEmbedPath(context.req.url || "");
+  const isEmbed = hasEmbedPath(context.resolvedUrl || context.req.url || "");
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ce75065 and 2547822.

📒 Files selected for processing (1)
  • apps/web/server/lib/router/getServerSideProps.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/web/server/lib/router/getServerSideProps.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/web/server/lib/router/getServerSideProps.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/web/server/lib/router/getServerSideProps.ts
🧬 Code graph analysis (1)
apps/web/server/lib/router/getServerSideProps.ts (1)
packages/features/routing-forms/lib/getRoutedUrl.ts (1)
  • hasEmbedPath (49-52)
🔇 Additional comments (3)
apps/web/server/lib/router/getServerSideProps.ts (3)

17-24: Props shape alignment looks good

Matches the TOO_MANY_REQUESTS branch ({ isEmbed, form: null, message: null, errorMessage }).


15-16: Remove req parameter for checkBotId: checkBotId() doesn’t accept or require the Next.js req object; your current usage in getServerSideProps is correct.


2-2: botid dependency and imports verified: botid is declared in apps/web/package.json (line 88), the server-only import (botid/server) is confined to server code, and the client bundle imports only botid/client/core.

});
}

if (process.env.VERCEL_BOTID_ENABLED === "true") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be prefixed by NEXT_PUBLIC so that it is available at client side.

) {
const isEmbed = hasEmbedPath(context.req.url || "");

if (process.env.NEXT_PUBLIC_IS_E2E && process.env.VERCEL_BOTID_ENABLED) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be !process.env.NEXT_PUBLIC_IS_E2E_&& process.env.VERCEL_BOTID_ENABLED

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
apps/web/next.config.js (1)

114-116: Env-gated injection looks good; add '1' support + server-side fallback and lazy require for consistency with other flags

  • Support "1" in addition to "true" (matches ORGANIZATIONS_ENABLED handling).
  • Accept server-side VERCEL_BOTID_ENABLED as a fallback to avoid requiring two env vars.
  • Lazy-require the plugin here to keep it optional.
-if (process.env.NEXT_PUBLIC_VERCEL_BOTID_ENABLED === "true") {
-  plugins.push(withBotId);
-}
+const isBotIdEnabled =
+  process.env.NEXT_PUBLIC_VERCEL_BOTID_ENABLED === "true" ||
+  process.env.NEXT_PUBLIC_VERCEL_BOTID_ENABLED === "1" ||
+  process.env.VERCEL_BOTID_ENABLED === "true" ||
+  process.env.VERCEL_BOTID_ENABLED === "1";
+
+if (isBotIdEnabled) {
+  const { withBotId } = require("botid/next/config");
+  plugins.push(withBotId);
+}

Run this quick check to verify consistent env usage and presence of the dependency:

#!/bin/bash
set -euo pipefail
echo "Scanning for BotID env vars…"
rg -n -C2 'NEXT_PUBLIC_VERCEL_BOTID_ENABLED|VERCEL_BOTID_ENABLED' \
  -g 'apps/web/**' -g '.env.example' -g 'turbo.json' || true
echo
echo "Checking BotID dependency in apps/web/package.json…"
rg -n '"botid"\s*:' apps/web/package.json || echo "Missing botid dep in apps/web/package.json"
🧹 Nitpick comments (1)
apps/web/next.config.js (1)

1-1: Lazy-load the BotID plugin to avoid unnecessary resolution and make the optional integration fully optional

Requiring the plugin at module scope forces resolution even when disabled. Lazy-require inside the env-gated block to reduce overhead and avoid failures if the dep is not present in some environments.

-const { withBotId } = require("botid/next/config");
+// BotID plugin is lazy-required when enabled (see block below).
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2547822 and 115b84c.

📒 Files selected for processing (3)
  • apps/web/instrumentation-client.ts (2 hunks)
  • apps/web/next.config.js (2 hunks)
  • apps/web/server/lib/router/getServerSideProps.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/web/instrumentation-client.ts
  • apps/web/server/lib/router/getServerSideProps.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/web/next.config.js
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Production builds / Build Web App
  • GitHub Check: Tests / Unit
  • GitHub Check: Type check / check-types

@emrysal
Copy link
Contributor

emrysal commented Sep 10, 2025

Closing in favour of isBot approach.

@emrysal emrysal closed this Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ❗️ .env changes contains changes to env variables foundation ready-for-e2e size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants