fix: PostHog cross-subdomain tracking and cookie consent#419
Conversation
WalkthroughAdds PostHog analytics across multiple apps (admin, web, docs, marketing, desktop): environment variables, Next.js rewrites for /ingest, PostHog initialization and consent handling, user-identification components, provider wiring, package dependencies, and CI/CD environment propagation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
apps/web/next.config.ts (1)
14-31: PostHog proxy configuration is correct.The rewrite rules and trailing slash configuration work as intended. Note that this configuration is duplicated across multiple apps—see the refactoring suggestion in
apps/marketing/next.config.tsreview.apps/admin/next.config.ts (1)
14-31: PostHog proxy configuration is correct.The rewrite rules and trailing slash configuration work as intended. Note that this configuration is duplicated across multiple apps—see the refactoring suggestion in
apps/marketing/next.config.tsreview.
🧹 Nitpick comments (10)
apps/marketing/package.json (1)
25-25: Consider updating to the latest PostHog version.The latest version of posthog-js is 1.306.2, which was released recently (Dec 15, 2025). Version 1.306.1 was released on Dec 13, 2025, so only a minor patch separates the two. Since the version constraint uses caret (
^1.306.1), patch updates will be included automatically during installation. For consistency with the rest of the ecosystem and to ensure you receive the latest bug fixes and improvements, consider bumping to^1.306.2.apps/marketing/next.config.ts (1)
15-30: Consider extracting duplicate rewrite configuration.The PostHog rewrite rules work correctly but are duplicated across
apps/admin/next.config.ts,apps/web/next.config.ts, andapps/marketing/next.config.ts.🔎 Suggested refactor to reduce duplication
Create a shared configuration file:
// packages/constants/src/posthog-rewrites.ts export const posthogRewrites = [ { source: "/ingest/static/:path*", destination: "https://us-assets.i.posthog.com/static/:path*", }, { source: "/ingest/:path*", destination: "https://us.i.posthog.com/:path*", }, { source: "/ingest/decide", destination: "https://us.i.posthog.com/decide", }, ];Then import and use in each Next.js config:
+import { posthogRewrites } from "@superset/constants/posthog-rewrites"; + const config: NextConfig = { // ... async rewrites() { - return [ - { - source: "/ingest/static/:path*", - destination: "https://us-assets.i.posthog.com/static/:path*", - }, - // ... - ]; + return posthogRewrites; }, };apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
28-32: Use name instead of email for person_display_name.Line 31 sets
person_display_nameto the email address, but you're already passing email on line 29. The display name should typically be the user's actual name for better readability in the PostHog dashboard.🔎 Suggested fix
if (posthogUser) { posthog.identify(posthogUser.id, { email: posthogUser.email, name: posthogUser.name, - person_display_name: posthogUser.email, + person_display_name: posthogUser.name || posthogUser.email, }); } else {apps/docs/src/env.ts (1)
17-17: Consider making NEXT_PUBLIC_POSTHOG_KEY optional for development.The PostHog key is currently required, which will cause build failures for contributors who don't have access to the analytics key. Consider using
.optional()or.default("")to allow development without PostHog configured.Proposed fix to make PostHog key optional
client: { NEXT_PUBLIC_COOKIE_DOMAIN: z.string().default("localhost"), - NEXT_PUBLIC_POSTHOG_KEY: z.string(), + NEXT_PUBLIC_POSTHOG_KEY: z.string().optional(), NEXT_PUBLIC_POSTHOG_HOST: z .string() .url()Note: You'll also need to update
apps/docs/src/instrumentation-client.tsto handle the optional key by guarding the initialization, similar to how the desktop app does.apps/marketing/src/env.ts (1)
19-24: Consider making NEXT_PUBLIC_POSTHOG_KEY optional for development.Similar to the docs app, the PostHog key is required, which may cause build failures for contributors without access to analytics credentials. Consider using
.optional()to allow development without PostHog configured.Proposed fix to make PostHog key optional
NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY: z.string(), NEXT_PUBLIC_COOKIE_DOMAIN: z.string().default("localhost"), - NEXT_PUBLIC_POSTHOG_KEY: z.string(), + NEXT_PUBLIC_POSTHOG_KEY: z.string().optional(), NEXT_PUBLIC_POSTHOG_HOST: z .string() .url()Note: The corresponding instrumentation-client.ts file will need a guard to skip initialization when the key is missing.
apps/web/src/instrumentation-client.ts (1)
14-15: Consider using a descriptive persistence_name instead of the cookie domain.Using
NEXT_PUBLIC_COOKIE_DOMAIN(e.g., "localhost" or "superset.sh") as thepersistence_nameis unconventional. PostHog's persistence_name is typically a descriptive identifier for the cookie/storage key (e.g., "posthog_web" or "superset_analytics"), not the domain itself.While this may work, it could cause confusion and makes the cookie name less meaningful. Consider using a constant like
"superset_posthog"instead.🔎 Suggested refactor
persistence: "cookie", - persistence_name: env.NEXT_PUBLIC_COOKIE_DOMAIN, + persistence_name: "superset_posthog",This makes the cookie name more meaningful and consistent across environments.
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
1-4: Consider using consistent alias imports.Line 4 uses a relative import
../../lib/posthogwhile line 2 uses an alias import. For consistency and easier refactoring, consider using an alias import if available.🔎 Proposed refactor
-import { posthog } from "../../lib/posthog"; +import { posthog } from "renderer/lib/posthog";Based on coding guidelines that recommend using aliases defined in
tsconfig.jsonwhen possible.apps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
49-56: Consider making the Accept button more prominent.Both the Opt-out and Accept buttons currently use
variant="outline"(lines 50, 53), which gives them equal visual weight. Typically, cookie consent banners make the Accept action more prominent to guide users toward the preferred choice.🔎 Suggested improvement
<div className="flex items-center gap-2"> <Button variant="outline" onClick={handleOptOut}> Opt-out </Button> - <Button variant="outline" onClick={handleAccept}> + <Button variant="default" onClick={handleAccept}> Accept </Button> </div>This provides clearer visual hierarchy and guides users toward accepting analytics while still offering the opt-out option.
apps/desktop/src/renderer/contexts/PostHogProvider.tsx (1)
18-23: Consider whether blocking app render on PostHog initialization is desired.The current implementation prevents rendering any children until PostHog is initialized (lines 18-21). This means the entire desktop app won't display any UI until analytics is ready.
For desktop applications, this could negatively impact perceived startup time. Consider whether PostHog initialization should be blocking or if the app could render immediately while analytics initializes in the background.
💡 Alternative approach
If non-blocking initialization is acceptable:
- // Don't render children until PostHog is initialized - if (!isInitialized) { - return null; - } - return <>{children}</>;The PostHogUserIdentifier component will handle waiting for analytics to be ready before identifying users, so blocking the entire app render may not be necessary.
apps/web/src/env.ts (1)
26-30: Consider making PostHog environment variables optional.
NEXT_PUBLIC_POSTHOG_KEY(line 26) is defined as required (z.string()with no.optional()), which means the app will fail to start if it's not set. This could hinder local development or environments where analytics isn't needed.The desktop app handles this gracefully with an optional check:
if (!POSTHOG_KEY) { console.log("[posthog] No key configured, skipping"); return; }🔎 Suggested refactor
- NEXT_PUBLIC_POSTHOG_KEY: z.string(), + NEXT_PUBLIC_POSTHOG_KEY: z.string().optional(), NEXT_PUBLIC_POSTHOG_HOST: z .string() .url() .default("https://us.i.posthog.com"),Then add a guard in the instrumentation client to skip initialization if the key is missing.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
AGENTS.md(1 hunks)apps/admin/next.config.ts(1 hunks)apps/admin/package.json(1 hunks)apps/admin/src/app/(dashboard)/layout.tsx(2 hunks)apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx(1 hunks)apps/admin/src/components/PostHogUserIdentifier/index.ts(1 hunks)apps/admin/src/env.ts(2 hunks)apps/admin/src/instrumentation-client.ts(1 hunks)apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx(1 hunks)apps/desktop/src/renderer/components/PostHogUserIdentifier/index.ts(1 hunks)apps/desktop/src/renderer/contexts/AppProviders.tsx(2 hunks)apps/desktop/src/renderer/contexts/PostHogProvider.tsx(1 hunks)apps/desktop/src/renderer/lib/posthog.ts(1 hunks)apps/docs/next.config.ts(1 hunks)apps/docs/package.json(1 hunks)apps/docs/src/env.ts(1 hunks)apps/docs/src/instrumentation-client.ts(1 hunks)apps/marketing/next.config.ts(1 hunks)apps/marketing/package.json(1 hunks)apps/marketing/src/app/layout.tsx(2 hunks)apps/marketing/src/components/CookieConsent/CookieConsent.tsx(1 hunks)apps/marketing/src/components/CookieConsent/index.ts(1 hunks)apps/marketing/src/env.ts(2 hunks)apps/marketing/src/instrumentation-client.ts(1 hunks)apps/marketing/src/lib/constants.ts(1 hunks)apps/web/next.config.ts(1 hunks)apps/web/package.json(1 hunks)apps/web/src/app/layout.tsx(2 hunks)apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx(1 hunks)apps/web/src/components/PostHogUserIdentifier/index.ts(1 hunks)apps/web/src/env.ts(2 hunks)apps/web/src/instrumentation-client.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/web/src/components/PostHogUserIdentifier/index.tsapps/web/src/instrumentation-client.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/docs/src/env.tsapps/marketing/next.config.tsapps/web/package.jsonapps/marketing/src/components/CookieConsent/index.tsapps/admin/next.config.tsapps/marketing/package.jsonapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/lib/posthog.tsapps/admin/src/env.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/components/PostHogUserIdentifier/index.tsapps/marketing/src/lib/constants.tsapps/docs/next.config.tsapps/web/next.config.tsapps/docs/src/instrumentation-client.tsapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/marketing/src/app/layout.tsxapps/admin/src/app/(dashboard)/layout.tsxapps/marketing/src/components/CookieConsent/CookieConsent.tsxapps/web/src/env.tsapps/desktop/src/renderer/contexts/PostHogProvider.tsxapps/web/src/app/layout.tsxapps/admin/package.jsonapps/docs/package.jsonapps/admin/src/instrumentation-client.tsapps/admin/src/components/PostHogUserIdentifier/index.tsapps/marketing/src/instrumentation-client.tsapps/marketing/src/env.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/web/src/components/PostHogUserIdentifier/index.tsapps/web/src/instrumentation-client.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/docs/src/env.tsapps/marketing/next.config.tsapps/marketing/src/components/CookieConsent/index.tsapps/admin/next.config.tsapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/lib/posthog.tsapps/admin/src/env.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/components/PostHogUserIdentifier/index.tsapps/marketing/src/lib/constants.tsapps/docs/next.config.tsapps/web/next.config.tsapps/docs/src/instrumentation-client.tsapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/marketing/src/app/layout.tsxapps/admin/src/app/(dashboard)/layout.tsxapps/marketing/src/components/CookieConsent/CookieConsent.tsxapps/web/src/env.tsapps/desktop/src/renderer/contexts/PostHogProvider.tsxapps/web/src/app/layout.tsxapps/admin/src/instrumentation-client.tsapps/admin/src/components/PostHogUserIdentifier/index.tsapps/marketing/src/instrumentation-client.tsapps/marketing/src/env.ts
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.{ts,tsx}: Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Use one component per file (no multi-component files)
Files:
apps/web/src/components/PostHogUserIdentifier/index.tsapps/marketing/src/components/CookieConsent/index.tsapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/components/PostHogUserIdentifier/index.tsapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/marketing/src/components/CookieConsent/CookieConsent.tsxapps/admin/src/components/PostHogUserIdentifier/index.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods from renderer process using window.ipcRenderer.invoke with type-safe object parameters
Files:
apps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/lib/posthog.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/components/PostHogUserIdentifier/index.tsapps/desktop/src/renderer/contexts/PostHogProvider.tsx
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx,js,jsx}: For Electron interprocess communication, ALWAYS use trpc as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Files:
apps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/lib/posthog.tsapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/components/PostHogUserIdentifier/index.tsapps/desktop/src/renderer/contexts/PostHogProvider.tsx
AGENTS.md
📄 CodeRabbit inference engine (CLAUDE.md)
Document agent responsibilities and interactions in AGENTS.md
Files:
AGENTS.md
🧠 Learnings (7)
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in apps/desktop/src/main/index.ts with override: true before any imports
Applied to files:
apps/docs/src/env.tsapps/desktop/src/renderer/lib/posthog.tsapps/admin/src/env.tsapps/docs/next.config.tsapps/web/src/env.ts
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to packages/ui/**/*.{ts,tsx} : Use React + TailwindCSS v4 + shadcn/ui for UI components in `packages/ui`
Applied to files:
apps/marketing/package.jsonAGENTS.mdapps/admin/package.jsonapps/docs/package.json
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to **/components/**/*.{ts,tsx} : Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Applied to files:
apps/docs/next.config.tsapps/docs/package.json
📚 Learning: 2025-11-24T21:32:21.725Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:21.725Z
Learning: Applies to apps/desktop/**/AGENTS.md : Document agent responsibilities, capabilities, and interaction patterns in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2025-11-24T21:32:17.800Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:17.800Z
Learning: Applies to AGENTS.md : Document agent responsibilities and interactions in AGENTS.md
Applied to files:
AGENTS.md
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to packages/db/src/**/*.{ts,tsx} : Use Drizzle ORM for all database operations in the `packages/db` package
Applied to files:
AGENTS.md
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to **/*.{ts,tsx,js,jsx,json} : Use Biome for code formatting and linting, running at root level for speed
Applied to files:
AGENTS.md
🧬 Code graph analysis (13)
apps/web/src/instrumentation-client.ts (1)
apps/web/src/env.ts (1)
env(5-47)
apps/desktop/src/renderer/contexts/AppProviders.tsx (1)
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(6-33)
apps/docs/src/env.ts (3)
apps/admin/src/env.ts (1)
env(5-44)apps/marketing/src/env.ts (1)
env(5-37)apps/web/src/env.ts (1)
env(5-47)
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (2)
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-39)apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-39)
apps/docs/src/instrumentation-client.ts (1)
apps/docs/src/env.ts (1)
env(5-33)
apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (2)
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-39)apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(6-33)
apps/marketing/src/app/layout.tsx (2)
apps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
CookieConsent(11-62)apps/marketing/src/components/CookieConsent/index.ts (1)
CookieConsent(1-1)
apps/admin/src/app/(dashboard)/layout.tsx (3)
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-39)apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(6-33)apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-39)
apps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
apps/marketing/src/lib/constants.ts (1)
ANALYTICS_CONSENT_KEY(1-1)
apps/desktop/src/renderer/contexts/PostHogProvider.tsx (2)
apps/desktop/src/renderer/contexts/index.ts (1)
PostHogProvider(3-3)apps/desktop/src/renderer/lib/posthog.ts (1)
initPostHog(7-32)
apps/web/src/app/layout.tsx (3)
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-39)apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(6-33)apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-39)
apps/admin/src/instrumentation-client.ts (4)
apps/admin/src/env.ts (1)
env(5-44)apps/docs/src/env.ts (1)
env(5-33)apps/marketing/src/env.ts (1)
env(5-37)apps/web/src/env.ts (1)
env(5-47)
apps/marketing/src/instrumentation-client.ts (3)
apps/docs/src/env.ts (1)
env(5-33)apps/marketing/src/env.ts (1)
env(5-37)apps/marketing/src/lib/constants.ts (1)
ANALYTICS_CONSENT_KEY(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Deploy Docs
- GitHub Check: Deploy API
- GitHub Check: Build
🔇 Additional comments (37)
apps/marketing/package.json (1)
13-32: Dependency integration looks good.The PostHog dependency is properly added alongside React 19 and Next.js 16. The version strategy (caret) is consistent with other dependencies in the file, and there are no apparent compatibility concerns with the existing stack.
AGENTS.md (1)
30-30: Clear Next.js 16 guidance.The documentation correctly highlights the middleware.ts → proxy.ts rename in Next.js 16 and provides clear instructions for developers.
Based on learnings, this follows the pattern of documenting important architectural decisions in AGENTS.md.
apps/admin/src/components/PostHogUserIdentifier/index.ts (1)
1-1: LGTM!The barrel export follows the project's component structure guidelines (one folder per component with index.ts export).
As per coding guidelines, this correctly implements the ComponentName/index.ts pattern.
apps/marketing/next.config.ts (1)
32-32: LGTM!The
skipTrailingSlashRedirectconfiguration is appropriate for the PostHog proxy setup.apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (2)
10-22: User object construction looks good.The fallback logic for deriving the user's name from available Clerk fields is well-structured and handles edge cases appropriately.
24-36: Clean effect implementation.The effect correctly handles the loading state and properly identifies or resets PostHog based on user presence.
apps/admin/src/instrumentation-client.ts (2)
8-8: Thedefaultsproperty is correctly named according to PostHog's documentation. This property accepts a date for a configuration snapshot used as defaults to initialize PostHog, and the value"2025-11-30"follows the documented format.
14-15: Remove cookie domain from persistence_name configuration.The persistence_name parameter should be a unique storage key identifier (like "superset_ph_admin"), not a domain name. Update the configuration:
cross_subdomain_cookie: true, persistence: "cookie", - persistence_name: env.NEXT_PUBLIC_COOKIE_DOMAIN, + persistence_name: "superset_ph_admin", loaded: (posthog) => {apps/admin/src/env.ts (1)
23-28: EnsureNEXT_PUBLIC_POSTHOG_KEYis configured in all deployment environments.In Zod, all properties are required by default. The
NEXT_PUBLIC_POSTHOG_KEYfield usesz.string()without a.default(), making it required at build time. Builds will fail if this environment variable is not set. Verify thatNEXT_PUBLIC_POSTHOG_KEYis configured in development, staging, and production deployment environments (e.g., Vercel project settings, CI/CD secrets). The NEXT_PUBLIC_POSTHOG_KEY must start with NEXT_PUBLIC_ to be accessible on the client-side.apps/admin/src/app/(dashboard)/layout.tsx (1)
16-16: LGTM! PostHog user identification integrated correctly.The PostHogUserIdentifier component is properly imported and placed within the layout hierarchy. The component will identify users via Clerk and sync with PostHog analytics.
Also applies to: 35-35
apps/docs/package.json (2)
16-17: LGTM! Dependencies correctly added for PostHog integration.The added dependencies (@t3-oss/env-nextjs, dotenv, posthog-js, zod) are appropriate for the PostHog analytics setup and environment configuration introduced in this PR.
Also applies to: 24-24, 28-28
27-27: Verify: three.js update appears unrelated to PR objectives.The update to three from an earlier version to ^0.181.2 doesn't seem related to PostHog cross-subdomain tracking. Please confirm whether this change was intentional or if it should be in a separate PR.
apps/desktop/src/renderer/lib/posthog.ts (3)
21-21: Confirm: localStorage persistence differs from PR's cross-subdomain cookie strategy.The desktop app uses
localStoragepersistence while the PR objectives mention configuring "persistence: 'cookie'" for cross-subdomain tracking across web apps. This difference is likely intentional since desktop apps don't need cross-subdomain tracking, but please confirm this is the desired behavior.
7-32: LGTM! Desktop PostHog initialization properly configured.The initialization correctly:
- Guards against missing API keys
- Disables automatic pageview/pageleave capture (appropriate for desktop)
- Uses localStorage persistence (suitable for Electron)
- Registers app metadata with platform information
- Enables debug mode in development
1-1: Import path is correct for Electron—no changes needed. Theposthog-js/dist/module.full.no-externalimport is required for Electron due to security restrictions about loading external code, and PostHog provides this full bundle specifically for this purpose. This is the recommended approach documented in PostHog's official Electron tutorial.apps/docs/src/env.ts (1)
5-33: LGTM! Environment configuration follows project patterns.The env configuration correctly:
- Extends Vercel presets for deployment
- Defines client-side PostHog variables with appropriate defaults
- Maps runtime environment variables
- Enables emptyStringAsUndefined for cleaner handling
- Follows the same structure as admin, web, and marketing apps
apps/docs/src/instrumentation-client.ts (1)
13-15: LGTM! Cookie-based cross-subdomain tracking correctly configured.The configuration properly implements cross-subdomain tracking by:
- Enabling
cross_subdomain_cookie: true- Using
persistence: "cookie"instead of localStorage- Setting
persistence_nameto the cookie domain for consistent identificationThis aligns with the PR objectives for cross-subdomain user tracking.
apps/docs/next.config.ts (3)
1-9: LGTM! Environment loading follows project standards.The dotenv configuration correctly loads environment variables from the monorepo root during development with override enabled, consistent with the project's environment loading pattern.
19-23: LGTM! Turbopack MDX resolution configured correctly.The turbopack alias properly resolves the MDX import source file for Nextra compatibility.
42-42: LGTM! skipTrailingSlashRedirect enables proper rewrite behavior.This setting prevents Next.js from redirecting
/ingestpaths with trailing slashes, ensuring the PostHog proxy rewrites work correctly.apps/marketing/src/env.ts (1)
5-37: LGTM! Environment configuration correctly extended for PostHog.The env configuration properly:
- Adds PostHog client variables with appropriate types
- Sets sensible defaults for cookie domain and PostHog host
- Maps runtime environment variables
- Maintains consistency with other apps in the monorepo
apps/admin/package.json (1)
29-29: No security issues identified for version ^1.306.1 of posthog-js.The latest version is 1.306.2, and version 1.306.1 was released on December 13, 2025. The only compromised posthog-js version during the Shai-Hulud attack was 1.297.3, and PostHog unpublished all relevant malicious versions and published newer clean versions. Version 1.306.1 is a clean release published after the incident remediation. The known XSS vulnerability (CVE-2023-32325) in versions prior to 1.57.2 has been patched, and 1.306.1 is well beyond this fix.
apps/desktop/src/renderer/contexts/AppProviders.tsx (1)
2-2: LGTM! Correct provider hierarchy and import pattern.The PostHogUserIdentifier component is correctly placed inside TRPCProvider (required for the
trpc.user.me.useQuery()hook) and follows the alias pattern from tsconfig.json.Also applies to: 15-15
apps/marketing/src/app/layout.tsx (1)
7-7: LGTM! Appropriate placement for cookie consent banner.The CookieConsent component is correctly placed after the Footer and inside ThemeProvider, allowing it to render as a fixed-position banner with theme support.
Also applies to: 62-62
apps/desktop/src/renderer/components/PostHogUserIdentifier/index.ts (1)
1-1: LGTM! Follows component structure guidelines.The barrel export pattern aligns with the coding guidelines for component organization.
apps/web/src/components/PostHogUserIdentifier/index.ts (1)
1-1: LGTM! Follows component structure guidelines.The barrel export pattern aligns with the coding guidelines for component organization.
apps/marketing/src/components/CookieConsent/index.ts (1)
1-1: LGTM! Follows component structure guidelines.The barrel export pattern aligns with the coding guidelines for component organization.
apps/web/src/app/layout.tsx (1)
7-7: LGTM! Correct provider hierarchy.The PostHogUserIdentifier component is correctly placed inside the Providers wrapper (which is inside ClerkProvider), ensuring the
useUser()hook from Clerk has the necessary context.Also applies to: 61-61
apps/web/src/instrumentation-client.ts (2)
8-8: No changes required. Thedefaults: "2025-11-30"syntax is correct according to PostHog's official documentation and is the intended usage for configuration snapshots.
5-22: The/ingestproxy rewrites are already properly configured innext.config.tswith all necessary paths (static assets, main API, and the decide endpoint) andskipTrailingSlashRedirect: trueis set. No further action required.apps/web/package.json (1)
31-31: Version 1.306.1 exists and is stable with no security vulnerabilities.Version 1.306.1 was released on December 13, 2025, and the current latest version is 1.306.2. There are no known security issues affecting the current package, and the project demonstrates healthy maintenance with regular releases.
apps/marketing/src/lib/constants.ts (1)
1-1: LGTM!The constant is well-defined with appropriate type narrowing using
as const, making it suitable for use as a localStorage key across the application.apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
6-33: LGTM!The component correctly integrates PostHog user identification with tRPC, following the same pattern as the web and admin apps. The implementation properly:
- Waits for the user query to complete before taking action
- Derives a normalized user object with memoization
- Identifies or resets PostHog based on authentication state
- Returns null as expected for a side-effect-only component
apps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
26-30: Opt-out handler works correctly but ties to instrumentation race condition.The
handleOptOutfunction correctly:
- Saves the user's preference to localStorage
- Immediately calls
posthog.opt_out_capturing()to stop trackingHowever, note that this relates to the race condition flagged in
apps/marketing/src/instrumentation-client.ts. If a user opts out, PostHog will stop capturing immediately, but any events captured between initialization and this opt-out action will have already been sent.Consider whether the timing of consent enforcement in the instrumentation file needs to be addressed to fully respect user privacy from the moment they decline.
apps/desktop/src/renderer/contexts/PostHogProvider.tsx (1)
13-16: Verify synchronous initialization assumption.The code calls
initPostHog()and immediately setsisInitializedtotruesynchronously (line 15). However, ifinitPostHog()performs any asynchronous operations or if PostHog's initialization is async, children may render before PostHog is fully ready.From the relevant code snippet,
apps/desktop/src/renderer/lib/posthog.tsshowsposthog.init()is called, which is typically synchronous, but theloadedcallback suggests there's an async aspect to initialization.Consider whether
initPostHogshould return a Promise and be awaited:🔎 Alternative implementation
useEffect(() => { - initPostHog(); - setIsInitialized(true); + async function initialize() { + await initPostHog(); + setIsInitialized(true); + } + initialize(); }, []);Or verify that
posthog.init()completes synchronously and theloadedcallback is only for registration, not readiness.apps/web/src/env.ts (1)
25-25: LGTM! Cookie domain default is appropriate for development.Setting
NEXT_PUBLIC_COOKIE_DOMAINto default to"localhost"aligns with the PR objectives and provides a sensible fallback for local development. In production, this should be overridden with the actual domain (e.g.,".superset.sh") for cross-subdomain tracking.apps/marketing/src/instrumentation-client.ts (1)
6-16: Remove invalidpersistence_nameconfiguration.Line 16 uses
persistence_name: env.NEXT_PUBLIC_COOKIE_DOMAIN, butpersistence_namedoes not appear to be a valid PostHog configuration option. PostHog's default cookie name isph_<project_api_key>_posthog. Remove this line or consult PostHog documentation for the correct configuration if cross-domain cookie control is needed. Thedefaultsconfiguration on line 9 is valid and requires no changes.Likely an incorrect or invalid review comment.
| async rewrites() { | ||
| return [ | ||
| { | ||
| source: "/ingest/static/:path*", | ||
| destination: "https://us-assets.i.posthog.com/static/:path*", | ||
| }, | ||
| { | ||
| source: "/ingest/:path*", | ||
| destination: "https://us.i.posthog.com/:path*", | ||
| }, | ||
| { | ||
| source: "/ingest/decide", | ||
| destination: "https://us.i.posthog.com/decide", | ||
| }, | ||
| ]; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove redundant /ingest/decide rewrite rule.
The /ingest/decide rewrite (lines 35-38) is redundant because the /ingest/:path* rewrite (lines 31-34) already matches and proxies all paths including /decide. Next.js will use the first matching rule, so the third rule will never be reached.
Proposed fix to remove redundant rule
async rewrites() {
return [
{
source: "/ingest/static/:path*",
destination: "https://us-assets.i.posthog.com/static/:path*",
},
{
source: "/ingest/:path*",
destination: "https://us.i.posthog.com/:path*",
},
- {
- source: "/ingest/decide",
- destination: "https://us.i.posthog.com/decide",
- },
];
},📝 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.
| async rewrites() { | |
| return [ | |
| { | |
| source: "/ingest/static/:path*", | |
| destination: "https://us-assets.i.posthog.com/static/:path*", | |
| }, | |
| { | |
| source: "/ingest/:path*", | |
| destination: "https://us.i.posthog.com/:path*", | |
| }, | |
| { | |
| source: "/ingest/decide", | |
| destination: "https://us.i.posthog.com/decide", | |
| }, | |
| ]; | |
| }, | |
| async rewrites() { | |
| return [ | |
| { | |
| source: "/ingest/static/:path*", | |
| destination: "https://us-assets.i.posthog.com/static/:path*", | |
| }, | |
| { | |
| source: "/ingest/:path*", | |
| destination: "https://us.i.posthog.com/:path*", | |
| }, | |
| ]; | |
| }, |
🤖 Prompt for AI Agents
In apps/docs/next.config.ts around lines 25 to 40 the explicit rewrite for
"/ingest/decide" is redundant because the earlier "/ingest/:path*" rule already
matches that path; remove the third rule (the "/ingest/decide" entry) from the
rewrites array so only the more general "/ingest/:path*" and
"/ingest/static/:path*" entries remain, preserving order and behavior.
| posthog.init(env.NEXT_PUBLIC_POSTHOG_KEY, { | ||
| api_host: "/ingest", | ||
| ui_host: "https://us.posthog.com", | ||
| defaults: "2025-11-30", | ||
| capture_pageview: "history_change", | ||
| capture_pageleave: true, | ||
| capture_exceptions: true, | ||
| debug: env.NODE_ENV === "development", | ||
| cross_subdomain_cookie: true, | ||
| persistence: "cookie", | ||
| persistence_name: env.NEXT_PUBLIC_COOKIE_DOMAIN, | ||
| loaded: (posthog) => { | ||
| posthog.register({ | ||
| app_name: "docs", | ||
| domain: window.location.hostname, | ||
| }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add guard for missing PostHog key.
The initialization will fail if NEXT_PUBLIC_POSTHOG_KEY is not provided. Add a guard similar to the desktop implementation to gracefully skip initialization when the key is missing.
Proposed fix to guard initialization
import posthog from "posthog-js";
import { env } from "@/env";
+if (!env.NEXT_PUBLIC_POSTHOG_KEY) {
+ console.log("[posthog] No key configured, skipping");
+} else {
posthog.init(env.NEXT_PUBLIC_POSTHOG_KEY, {
api_host: "/ingest",
ui_host: "https://us.posthog.com",
defaults: "2025-11-30",
capture_pageview: "history_change",
capture_pageleave: true,
capture_exceptions: true,
debug: env.NODE_ENV === "development",
cross_subdomain_cookie: true,
persistence: "cookie",
persistence_name: env.NEXT_PUBLIC_COOKIE_DOMAIN,
loaded: (posthog) => {
posthog.register({
app_name: "docs",
domain: window.location.hostname,
});
},
});
+}🤖 Prompt for AI Agents
In apps/docs/src/instrumentation-client.ts around lines 5 to 22, the code calls
posthog.init unconditionally which will throw if NEXT_PUBLIC_POSTHOG_KEY is
missing; add a guard that checks env.NEXT_PUBLIC_POSTHOG_KEY (or
process.env.NEXT_PUBLIC_POSTHOG_KEY depending on how env is referenced in this
file) and only calls posthog.init when that key is truthy, otherwise skip
initialization (no-op) and return early; keep the rest of the init options
unchanged and ensure the guard mirrors the desktop implementation pattern used
elsewhere.
| loaded: (posthog) => { | ||
| posthog.register({ | ||
| app_name: "marketing", | ||
| domain: window.location.hostname, | ||
| }); | ||
|
|
||
| const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); | ||
| if (consent === "declined") { | ||
| posthog.opt_out_capturing(); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Race condition: Consent enforcement happens too late.
The consent check occurs inside the loaded callback (line 23-26), which executes after PostHog initialization. This creates a window where PostHog may capture events before opt_out_capturing() is called, potentially violating user consent preferences.
For proper consent enforcement, check localStorage before calling posthog.init():
🔎 Proposed fix
+const consent = typeof window !== 'undefined'
+ ? localStorage.getItem(ANALYTICS_CONSENT_KEY)
+ : null;
+
+if (consent === "declined") {
+ // Don't initialize PostHog at all if user declined
+} else {
posthog.init(env.NEXT_PUBLIC_POSTHOG_KEY, {
api_host: "/ingest",
ui_host: "https://us.posthog.com",
defaults: "2025-11-30",
capture_pageview: "history_change",
capture_pageleave: true,
capture_exceptions: true,
debug: env.NODE_ENV === "development",
cross_subdomain_cookie: true,
persistence: "cookie",
persistence_name: env.NEXT_PUBLIC_COOKIE_DOMAIN,
loaded: (posthog) => {
posthog.register({
app_name: "marketing",
domain: window.location.hostname,
});
-
- const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY);
- if (consent === "declined") {
- posthog.opt_out_capturing();
- }
},
});
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/marketing/src/instrumentation-client.ts around lines 17 to 27, the
consent check currently runs in the posthog.loaded callback which occurs after
posthog.init, creating a race where events may be sent before opt_out_capturing
is applied; move the localStorage consent read to execute before calling
posthog.init and, if consent === "declined", call posthog.opt_out_capturing()
immediately (before init) so PostHog never begins capturing, otherwise proceed
to init and register as before.
| export function PostHogUserIdentifier() { | ||
| const { user, isLoaded } = useUser(); | ||
|
|
||
| const posthogUser = useMemo(() => { | ||
| if (!user) return null; | ||
| const name = | ||
| user.fullName || | ||
| [user.firstName, user.lastName].filter(Boolean).join(" ") || | ||
| user.primaryEmailAddress?.emailAddress || | ||
| ""; | ||
| return { | ||
| id: user.id, | ||
| email: user.primaryEmailAddress?.emailAddress ?? "", | ||
| name, | ||
| }; | ||
| }, [user]); | ||
|
|
||
| useEffect(() => { | ||
| if (!isLoaded) return; | ||
|
|
||
| if (posthogUser) { | ||
| posthog.identify(posthogUser.id, { | ||
| email: posthogUser.email, | ||
| name: posthogUser.name, | ||
| person_display_name: posthogUser.email, | ||
| }); | ||
| } else { | ||
| posthog.reset(); | ||
| } | ||
| }, [posthogUser, isLoaded]); | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated PostHogUserIdentifier to shared package.
This component is identical to the one in apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx. Since both the web and admin apps use Clerk for authentication, this component can be extracted to a shared package to follow DRY principles.
💡 Suggested approach
- Create
packages/posthog/PostHogUserIdentifier.tsx(or similar shared location) - Export the component from the shared package
- Import it in both web and admin apps
This will:
- Eliminate code duplication
- Ensure consistent behavior across apps
- Make future updates easier to maintain
- Reduce the chance of divergence between implementations
🤖 Prompt for AI Agents
In apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
(lines 7-39) this component is identical to
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx;
extract it to a shared package (e.g.,
packages/posthog/PostHogUserIdentifier.tsx) and export it from that package,
then replace the local component in both apps with imports from the shared
package. Concretely: create the shared file with the same component, add an
export in the package's entry (and package.json) so it is consumable by
workspaces, update both apps to import the component from the new package path,
remove the duplicated files from the apps, and run the workspace install/build
to ensure imports resolve. Ensure any toolchain/tsconfig paths or build configs
are updated if necessary.
- Add persistence: "cookie" and persistence_name to all apps for proper cross-subdomain tracking (matching monorepo pattern) - Default NEXT_PUBLIC_COOKIE_DOMAIN to "localhost" for development - Refactor PostHogUserIdentifier to use Clerk internally (no props) - Move PostHogUserIdentifier to root layout (web) and dashboard layout (admin) - Update marketing cookie banner to Trigger.dev-style (bottom-left, opt-out model) - Add Next.js 16 proxy.ts note to AGENTS.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
saddlepaddle
force-pushed
the
given-constrictor-6c1fe1
branch
from
3210d19 to
2030810
Compare
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
7-24: Acknowledged: Code duplication with admin app.As noted in the previous review, this component is identical to
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx. Both implementations use Clerk'suseUserhook and share the exact same logic.The previous review's suggestion to extract this to a shared package (e.g.,
packages/posthog/PostHogUserIdentifier.tsx) remains valid and would eliminate this duplication.apps/admin/next.config.ts (1)
24-27: Remove redundant /ingest/decide rewrite rule.As noted in a previous review, this rewrite is redundant because
/ingest/:path*already matches all paths including/decide. The more specific rule will never be reached.🔎 Proposed fix
async rewrites() { return [ { source: "/ingest/static/:path*", destination: "https://us-assets.i.posthog.com/static/:path*", }, { source: "/ingest/:path*", destination: "https://us.i.posthog.com/:path*", }, - { - source: "/ingest/decide", - destination: "https://us.i.posthog.com/decide", - }, ]; },apps/docs/next.config.ts (1)
35-38: Remove redundant /ingest/decide rewrite rule.This rewrite is redundant because
/ingest/:path*(lines 31-34) already matches all paths including/decide. Next.js will use the first matching rule, so this specific rule will never be reached.🔎 Proposed fix
async rewrites() { return [ { source: "/ingest/static/:path*", destination: "https://us-assets.i.posthog.com/static/:path*", }, { source: "/ingest/:path*", destination: "https://us.i.posthog.com/:path*", }, - { - source: "/ingest/decide", - destination: "https://us.i.posthog.com/decide", - }, ]; },
🧹 Nitpick comments (6)
apps/web/src/instrumentation-client.ts (1)
5-22: Consider adding error handling for PostHog initialization.While the env validation ensures
NEXT_PUBLIC_POSTHOG_KEYis present, adding defensive error handling around PostHog initialization would improve resilience against runtime failures (network issues, blocked third-party scripts, etc.).🔎 Optional: Add try-catch for initialization
-posthog.init(env.NEXT_PUBLIC_POSTHOG_KEY, { - api_host: "/ingest", - ui_host: "https://us.posthog.com", - defaults: "2025-11-30", - capture_pageview: "history_change", - capture_pageleave: true, - capture_exceptions: true, - debug: env.NODE_ENV === "development", - cross_subdomain_cookie: true, - persistence: "cookie", - persistence_name: env.NEXT_PUBLIC_COOKIE_DOMAIN, - loaded: (posthog) => { - posthog.register({ - app_name: "web", - domain: window.location.hostname, - }); - }, -}); +try { + posthog.init(env.NEXT_PUBLIC_POSTHOG_KEY, { + api_host: "/ingest", + ui_host: "https://us.posthog.com", + defaults: "2025-11-30", + capture_pageview: "history_change", + capture_pageleave: true, + capture_exceptions: true, + debug: env.NODE_ENV === "development", + cross_subdomain_cookie: true, + persistence: "cookie", + persistence_name: env.NEXT_PUBLIC_COOKIE_DOMAIN, + loaded: (posthog) => { + posthog.register({ + app_name: "web", + domain: window.location.hostname, + }); + }, + }); +} catch (error) { + console.error("Failed to initialize PostHog:", error); +}apps/web/package.json (1)
31-31: Consider updating posthog-js to the latest available version.The latest version is 1.306.2, published 17 hours ago. Version 1.306.1 was released on December 13, 2025, which is after the November 24, 2025 Shai-Hulud 2.0 attack on PostHog packages, with malicious versions identified and deleted by 9:30 AM UTC. The specified version ^1.306.1 is valid and secure, but upgrading to 1.306.2 would ensure the most recent patches and improvements are included.
apps/admin/src/instrumentation-client.ts (1)
14-15: Consider using a descriptive cookie name instead of the domain.Using
env.NEXT_PUBLIC_COOKIE_DOMAIN(which defaults to "localhost" in development) as thepersistence_namecreates a cookie named "localhost" or the domain value. PostHog'spersistence_nameshould typically be a descriptive identifier for the cookie (e.g., "ph_superset_admin") rather than the domain itself.🔎 Suggested fix
persistence: "cookie", - persistence_name: env.NEXT_PUBLIC_COOKIE_DOMAIN, + persistence_name: "ph_superset_admin",The
cross_subdomain_cookieoption combined with proper cookie domain configuration will handle cross-subdomain behavior without needing the domain in the cookie name.apps/docs/next.config.ts (1)
18-23: Turbopack MDX configuration appears unrelated to PostHog integration.The
turbopack.resolveAliasconfiguration for Nextra's MDX resolution is included in this PR but appears unrelated to the PostHog integration changes. Consider whether this should be in a separate commit or PR for clarity.apps/marketing/src/components/CookieConsent/CookieConsent.tsx (2)
32-61: Consider adding accessibility attributes.The consent banner could benefit from semantic markup to improve screen reader experience:
- Add
role="dialog"orrole="alert"to the banner- Add
aria-labelledbyoraria-labelfor screen reader identification- Consider focus management when the banner appears
🔎 Example with improved accessibility
<motion.div initial={{ y: 20, opacity: 0 }} animate={{ y: 0, opacity: 1 }} exit={{ y: 20, opacity: 0 }} transition={{ type: "spring", damping: 25, stiffness: 300 }} className="fixed bottom-4 left-4 z-50 max-w-xs rounded-lg border border-border bg-card p-4 shadow-lg" + role="dialog" + aria-label="Cookie consent" + aria-describedby="cookie-consent-text" > - <p className="text-sm text-muted-foreground"> + <p id="cookie-consent-text" className="text-sm text-muted-foreground"> We only collect analytics cookies so we can improve your experience. </p>
11-11: Add TypeScript return type annotation.As per coding guidelines, prioritize type safety by adding an explicit return type annotation.
🔎 Suggested improvement
-export function CookieConsent() { +export function CookieConsent(): JSX.Element {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (35)
AGENTS.md(1 hunks)apps/admin/next.config.ts(1 hunks)apps/admin/package.json(1 hunks)apps/admin/src/app/providers.tsx(1 hunks)apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx(1 hunks)apps/admin/src/components/PostHogUserIdentifier/index.ts(1 hunks)apps/admin/src/env.ts(2 hunks)apps/admin/src/instrumentation-client.ts(1 hunks)apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx(1 hunks)apps/desktop/src/renderer/components/PostHogUserIdentifier/index.ts(1 hunks)apps/desktop/src/renderer/contexts/AppProviders.tsx(2 hunks)apps/desktop/src/renderer/contexts/PostHogProvider.tsx(1 hunks)apps/desktop/src/renderer/lib/posthog.ts(1 hunks)apps/docs/next.config.ts(1 hunks)apps/docs/package.json(1 hunks)apps/docs/src/app/layout.tsx(2 hunks)apps/docs/src/app/providers.tsx(1 hunks)apps/docs/src/env.ts(1 hunks)apps/docs/src/instrumentation-client.ts(1 hunks)apps/marketing/next.config.ts(1 hunks)apps/marketing/package.json(1 hunks)apps/marketing/src/app/layout.tsx(2 hunks)apps/marketing/src/app/providers.tsx(1 hunks)apps/marketing/src/components/CookieConsent/CookieConsent.tsx(1 hunks)apps/marketing/src/components/CookieConsent/index.ts(1 hunks)apps/marketing/src/env.ts(2 hunks)apps/marketing/src/instrumentation-client.ts(1 hunks)apps/marketing/src/lib/constants.ts(1 hunks)apps/web/next.config.ts(1 hunks)apps/web/package.json(1 hunks)apps/web/src/app/providers.tsx(1 hunks)apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx(1 hunks)apps/web/src/components/PostHogUserIdentifier/index.ts(1 hunks)apps/web/src/env.ts(2 hunks)apps/web/src/instrumentation-client.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (19)
- apps/web/next.config.ts
- apps/marketing/src/components/CookieConsent/index.ts
- apps/marketing/next.config.ts
- apps/web/src/env.ts
- apps/marketing/src/lib/constants.ts
- apps/desktop/src/renderer/contexts/AppProviders.tsx
- apps/marketing/src/app/layout.tsx
- apps/docs/src/env.ts
- apps/web/src/components/PostHogUserIdentifier/index.ts
- apps/docs/package.json
- apps/marketing/src/instrumentation-client.ts
- apps/docs/src/instrumentation-client.ts
- apps/marketing/src/env.ts
- AGENTS.md
- apps/marketing/package.json
- apps/admin/package.json
- apps/desktop/src/renderer/contexts/PostHogProvider.tsx
- apps/desktop/src/renderer/lib/posthog.ts
- apps/desktop/src/renderer/components/PostHogUserIdentifier/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/docs/src/app/providers.tsxapps/admin/next.config.tsapps/admin/src/app/providers.tsxapps/marketing/src/app/providers.tsxapps/admin/src/instrumentation-client.tsapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/web/src/app/providers.tsxapps/admin/src/env.tsapps/admin/src/components/PostHogUserIdentifier/index.tsapps/docs/next.config.tsapps/docs/src/app/layout.tsxapps/web/package.jsonapps/web/src/instrumentation-client.tsapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/marketing/src/components/CookieConsent/CookieConsent.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/docs/src/app/providers.tsxapps/admin/next.config.tsapps/admin/src/app/providers.tsxapps/marketing/src/app/providers.tsxapps/admin/src/instrumentation-client.tsapps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/web/src/app/providers.tsxapps/admin/src/env.tsapps/admin/src/components/PostHogUserIdentifier/index.tsapps/docs/next.config.tsapps/docs/src/app/layout.tsxapps/web/src/instrumentation-client.tsapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/marketing/src/components/CookieConsent/CookieConsent.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.{ts,tsx}: Structure project folders as one folder per component with PascalCase naming (ComponentName/ComponentName.tsx + index.ts barrel export)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Use one component per file (no multi-component files)
Files:
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/admin/src/components/PostHogUserIdentifier/index.tsapps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsxapps/marketing/src/components/CookieConsent/CookieConsent.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods from renderer process using window.ipcRenderer.invoke with type-safe object parameters
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx,js,jsx}: For Electron interprocess communication, ALWAYS use trpc as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Files:
apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to **/components/**/*.{ts,tsx} : Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Applied to files:
apps/docs/next.config.ts
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in apps/desktop/src/main/index.ts with override: true before any imports
Applied to files:
apps/docs/next.config.ts
🧬 Code graph analysis (8)
apps/docs/src/app/providers.tsx (5)
apps/admin/src/app/providers.tsx (1)
Providers(13-31)apps/marketing/src/app/providers.tsx (1)
Providers(8-22)apps/web/src/app/providers.tsx (1)
Providers(13-31)apps/desktop/src/renderer/contexts/PostHogProvider.tsx (1)
PostHogProvider(10-24)apps/desktop/src/renderer/lib/posthog.ts (1)
posthog(34-34)
apps/admin/src/app/providers.tsx (7)
apps/docs/src/app/providers.tsx (1)
Providers(6-8)apps/web/src/app/providers.tsx (1)
Providers(13-31)apps/desktop/src/renderer/contexts/PostHogProvider.tsx (1)
PostHogProvider(10-24)packages/shared/src/constants.ts (1)
THEME_STORAGE_KEY(20-20)apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-24)apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(6-23)apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-24)
apps/marketing/src/app/providers.tsx (5)
apps/admin/src/app/providers.tsx (1)
Providers(13-31)apps/docs/src/app/providers.tsx (1)
Providers(6-8)apps/web/src/app/providers.tsx (1)
Providers(13-31)apps/desktop/src/renderer/contexts/PostHogProvider.tsx (1)
PostHogProvider(10-24)packages/shared/src/constants.ts (1)
THEME_STORAGE_KEY(20-20)
apps/admin/src/instrumentation-client.ts (1)
apps/admin/src/env.ts (1)
env(5-44)
apps/web/src/app/providers.tsx (5)
apps/docs/src/app/providers.tsx (1)
Providers(6-8)apps/marketing/src/app/providers.tsx (1)
Providers(8-22)apps/desktop/src/renderer/contexts/PostHogProvider.tsx (1)
PostHogProvider(10-24)apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-24)apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-24)
apps/docs/src/app/layout.tsx (4)
apps/admin/src/app/providers.tsx (1)
Providers(13-31)apps/docs/src/app/providers.tsx (1)
Providers(6-8)apps/marketing/src/app/providers.tsx (1)
Providers(8-22)apps/web/src/app/providers.tsx (1)
Providers(13-31)
apps/web/src/instrumentation-client.ts (2)
apps/desktop/src/renderer/lib/posthog.ts (1)
posthog(34-34)apps/web/src/env.ts (1)
env(5-47)
apps/web/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (2)
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(7-24)apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(6-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Deploy API
- GitHub Check: Deploy Docs
- GitHub Check: Build
🔇 Additional comments (13)
apps/web/src/instrumentation-client.ts (2)
1-3: LGTM!The imports are correct and necessary for PostHog client-side initialization.
8-8: No issues found—defaultsconfiguration is valid.The
defaultsconfiguration accepts a date string (such as 2025-11-30) for a configuration snapshot used as defaults to initialize PostHog. This default is overridden when you explicitly set a value for any of the options.apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
6-23: LGTM! Correct use of tRPC and effect for PostHog identification.The component correctly uses tRPC as specified in the coding guidelines for Electron interprocess communication. The useEffect hook is appropriate here since it handles a necessary side effect (identifying or resetting the user in PostHog based on authentication state).
apps/marketing/src/app/providers.tsx (1)
8-22: LGTM! Consistent provider structure across apps.The Providers component correctly wraps PostHogProvider around ThemeProvider, following the same pattern used in the docs, admin, and web apps. The implementation is clean and maintainable.
apps/docs/src/app/providers.tsx (1)
6-8: LGTM! Clean and minimal provider for docs app.The Providers component is appropriately minimal for the docs app, wrapping only PostHogProvider without the additional complexity of user identification or theming that may not be needed in documentation.
apps/admin/src/components/PostHogUserIdentifier/index.ts (1)
1-1: LGTM! Follows component structure guidelines.The barrel export correctly follows the coding guidelines for component folder structure (ComponentName/index.ts).
apps/docs/src/app/layout.tsx (1)
36-46: LGTM! Proper provider integration.The Layout is correctly wrapped with the Providers component, ensuring PostHog analytics context is available throughout the docs app.
apps/web/src/app/providers.tsx (1)
15-29: LGTM! Proper provider nesting and user identification.The Providers component correctly integrates PostHog as the outermost provider, with PostHogUserIdentifier properly placed inside the ThemeProvider to ensure user data is available when identifying to PostHog. The structure matches the admin app implementation.
apps/admin/src/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
7-24: LGTM: Clean user identification implementation.The component correctly:
- Waits for Clerk to load before identifying
- Extracts user data safely with optional chaining
- Resets PostHog when user logs out
- Returns null (no UI rendering)
This follows the same pattern used in the web app and aligns with PostHog best practices.
apps/admin/src/env.ts (1)
23-28: LGTM: Environment configuration is well-structured.The PostHog environment variables are properly configured:
NEXT_PUBLIC_POSTHOG_KEYis required (no default)NEXT_PUBLIC_POSTHOG_HOSThas sensible defaultNEXT_PUBLIC_COOKIE_DOMAINdefaults to "localhost" for local development- URL validation ensures proper format
apps/admin/src/instrumentation-client.ts (1)
8-8: Thedefaultsparameter correctly uses a date string format.The
defaultsis a date, such as 2025-11-30, for a configuration snapshot used as defaults to initialize PostHog. The value"2025-11-30"is the proper format for this configuration option.Likely an incorrect or invalid review comment.
apps/admin/src/app/providers.tsx (1)
6-7: Rename and relocate providers file to match Next.js conventions.The
instrumentation-client.tsfile should contain only side-effects and monitoring code that runs before the application becomes interactive, not export React components. The current file exports aProviderscomponent and should be renamed toproviders.tsx. Additionally, it should be placed in the root of the application or inside a src folder, not insrc/app/.apps/marketing/src/components/CookieConsent/CookieConsent.tsx (1)
1-10: LGTM! Clean imports and correct client directive.The "use client" directive is necessary for this component due to the use of hooks and event handlers. All imports are utilized appropriately.
| useEffect(() => { | ||
| const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); | ||
| if (consent === null) { | ||
| setShowBanner(true); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Add error handling for localStorage access.
localStorage can throw a SecurityError in private browsing modes or when storage is disabled. Without error handling, the component could crash on mount in these scenarios.
🔎 Recommended fix with try-catch wrapper
useEffect(() => {
+ try {
const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY);
if (consent === null) {
setShowBanner(true);
}
+ } catch (error) {
+ // Silently fail if localStorage is unavailable
+ console.warn('localStorage is not available:', error);
+ }
}, []);📝 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.
| useEffect(() => { | |
| const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); | |
| if (consent === null) { | |
| setShowBanner(true); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| try { | |
| const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); | |
| if (consent === null) { | |
| setShowBanner(true); | |
| } | |
| } catch (error) { | |
| // Silently fail if localStorage is unavailable | |
| console.warn('localStorage is not available:', error); | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
In apps/marketing/src/components/CookieConsent/CookieConsent.tsx around lines 14
to 19, accessing localStorage.getItem can throw (e.g., SecurityError in private
mode) and may crash the component; wrap the localStorage access in a try-catch
and first guard that window and window.localStorage exist, then call getItem
inside the try block, treat any thrown error or missing storage as a
null/unknown consent (so setShowBanner(true)), and optionally log a warning to
console or a logger; do not change component behavior beyond adding this safe
access pattern.
| const handleAccept = () => { | ||
| localStorage.setItem(ANALYTICS_CONSENT_KEY, "accepted"); | ||
| setShowBanner(false); | ||
| }; |
There was a problem hiding this comment.
Add error handling for localStorage in handleAccept.
Similar to the useEffect, localStorage.setItem can throw exceptions in restricted environments. The handler should gracefully handle these errors.
🔎 Recommended fix with error handling
const handleAccept = () => {
- localStorage.setItem(ANALYTICS_CONSENT_KEY, "accepted");
- setShowBanner(false);
+ try {
+ localStorage.setItem(ANALYTICS_CONSENT_KEY, "accepted");
+ setShowBanner(false);
+ } catch (error) {
+ console.warn('Failed to save consent preference:', error);
+ // Still hide banner to avoid blocking user
+ setShowBanner(false);
+ }
};🤖 Prompt for AI Agents
In apps/marketing/src/components/CookieConsent/CookieConsent.tsx around lines 21
to 24, the handleAccept function calls localStorage.setItem which can throw in
restricted environments; wrap the localStorage.setItem call in a try/catch, keep
setShowBanner(false) outside or in finally so the banner still hides even if
storage fails, and in the catch log a non-fatal warning (console.warn or the app
logger) describing the failure and the key/value that failed to persist.
| const handleOptOut = () => { | ||
| localStorage.setItem(ANALYTICS_CONSENT_KEY, "declined"); | ||
| posthog.opt_out_capturing(); | ||
| setShowBanner(false); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the CookieConsent component implementation
echo "=== CookieConsent component content ==="
cat -n apps/marketing/src/components/CookieConsent/CookieConsent.tsx 2>/dev/null | head -50
echo -e "\n=== Check for posthog initialization locations ==="
rg -l "posthog.*init" --type ts --type tsx --max-count 10
echo -e "\n=== Check where CookieConsent is imported ==="
rg "CookieConsent" --type ts --type tsx -B 2 -A 2 --max-count 20
echo -e "\n=== Check instrumentation files ==="
find . -name "*instrumentation*" -type f 2>/dev/null | head -10Repository: superset-sh/superset
Length of output: 2400
Add error handling for localStorage and PostHog method calls.
The handleOptOut function and other localStorage operations lack error handling. localStorage.setItem throws when JavaScript attempts to store data but encounters storage limitations, such as storage quota exceeded or private browsing restrictions. Additionally, the component directly imports posthog (line 6), and PostHog documentation states this will likely cause errors as the library might not be initialized yet.
Apply error handling to all localStorage and PostHog operations:
const handleOptOut = () => {
- localStorage.setItem(ANALYTICS_CONSENT_KEY, "declined");
- posthog.opt_out_capturing();
- setShowBanner(false);
+ try {
+ localStorage.setItem(ANALYTICS_CONSENT_KEY, "declined");
+ } catch (error) {
+ console.warn('Failed to save consent preference:', error);
+ }
+
+ try {
+ if (posthog && typeof posthog.opt_out_capturing === 'function') {
+ posthog.opt_out_capturing();
+ }
+ } catch (error) {
+ console.warn('Failed to opt out of PostHog tracking:', error);
+ }
+
+ setShowBanner(false);
};Also wrap localStorage calls in useEffect (line 15) and handleAccept (line 22) with try-catch blocks.
🤖 Prompt for AI Agents
In apps/marketing/src/components/CookieConsent/CookieConsent.tsx around lines 26
to 30, handleOptOut directly writes to localStorage and calls posthog without
error handling or initialization checks; add try-catch around the
localStorage.setItem and posthog call to catch quota/private-mode errors and any
posthog runtime errors, and guard the posthog call by checking it is initialized
and the opt_out_capturing function exists before invoking it; additionally, wrap
all other localStorage uses (the useEffect at ~line 15 and handleAccept at ~line
22) in try-catch blocks and perform similar guarded checks before calling any
posthog methods so storage failures or uninitialized PostHog do not throw.
- Add POSTHOG_COOKIE_NAME constant in shared package - Update all instrumentation-client.ts to use shared constant - Remove NEXT_PUBLIC_POSTHOG_HOST default (now required) - Remove NEXT_PUBLIC_COOKIE_DOMAIN from docs (unused) - Add @superset/shared dependency to docs - Pass all env.ts vars to --env in deploy workflows 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/deploy-production.yml (1)
289-303: Inconsistent environment variable propagation in docs deployment.The deploy-docs job has
NEXT_PUBLIC_API_URL,NEXT_PUBLIC_WEB_URL, andNEXT_PUBLIC_MARKETING_URLin the build environment (lines 293-295), but these variables are missing from thevercel deploy --envcommand (lines 301-303), which only includes the PostHog variables.All other deployments (web, marketing, admin) propagate all their
NEXT_PUBLIC_*variables to thevercel deploycommand. This inconsistency could cause the docs app to have missing environment variables at runtime.🔎 Suggested fix to add missing variables to deploy command
vercel deploy --prod --prebuilt --token=$VERCEL_TOKEN \ + --env NEXT_PUBLIC_API_URL=$NEXT_PUBLIC_API_URL \ + --env NEXT_PUBLIC_WEB_URL=$NEXT_PUBLIC_WEB_URL \ + --env NEXT_PUBLIC_MARKETING_URL=$NEXT_PUBLIC_MARKETING_URL \ --env NEXT_PUBLIC_POSTHOG_KEY=$NEXT_PUBLIC_POSTHOG_KEY \ --env NEXT_PUBLIC_POSTHOG_HOST=$NEXT_PUBLIC_POSTHOG_HOST
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.github/workflows/deploy-preview.yml(5 hunks).github/workflows/deploy-production.yml(5 hunks)apps/admin/src/env.ts(2 hunks)apps/admin/src/instrumentation-client.ts(1 hunks)apps/docs/package.json(1 hunks)apps/docs/src/env.ts(1 hunks)apps/docs/src/instrumentation-client.ts(1 hunks)apps/marketing/src/env.ts(2 hunks)apps/marketing/src/instrumentation-client.ts(1 hunks)apps/web/src/env.ts(2 hunks)apps/web/src/instrumentation-client.ts(1 hunks)packages/shared/src/constants.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/web/src/env.ts
- apps/docs/src/env.ts
- apps/web/src/instrumentation-client.ts
- apps/marketing/src/instrumentation-client.ts
- apps/docs/src/instrumentation-client.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for code formatting and linting, running at root level for speed
Files:
apps/admin/src/env.tspackages/shared/src/constants.tsapps/admin/src/instrumentation-client.tsapps/marketing/src/env.tsapps/docs/package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid
anytype and prioritize type safety in TypeScript code
Files:
apps/admin/src/env.tspackages/shared/src/constants.tsapps/admin/src/instrumentation-client.tsapps/marketing/src/env.ts
🧠 Learnings (2)
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to **/components/**/*.{ts,tsx} : Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Applied to files:
apps/docs/package.json
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to packages/ui/**/*.{ts,tsx} : Use React + TailwindCSS v4 + shadcn/ui for UI components in `packages/ui`
Applied to files:
apps/docs/package.json
🧬 Code graph analysis (1)
apps/admin/src/instrumentation-client.ts (2)
apps/admin/src/env.ts (1)
env(5-41)packages/shared/src/constants.ts (1)
POSTHOG_COOKIE_NAME(36-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Admin
- GitHub Check: Deploy API
- GitHub Check: Build
🔇 Additional comments (12)
.github/workflows/deploy-production.yml (4)
136-137: LGTM! PostHog variables correctly configured.The PostHog environment variables are properly added to both the build environment and the Vercel deployment command, maintaining consistency with other
NEXT_PUBLIC_*variables.Also applies to: 152-153
192-193: LGTM! PostHog variables correctly configured.The PostHog environment variables are properly added to both the build environment and the Vercel deployment command.
Also applies to: 203-204
245-246: LGTM! PostHog variables correctly configured.The PostHog environment variables are properly added to both the build environment and the Vercel deployment command.
Also applies to: 258-259
69-94: Verify whether API deployment should include PostHog variables.The deploy-api job does not include
NEXT_PUBLIC_POSTHOG_KEYorNEXT_PUBLIC_POSTHOG_HOSTenvironment variables, while all other deployments (web, marketing, admin, docs) do.Since PostHog is typically a client-side analytics tool and the API is backend-only, this omission may be intentional. However, please confirm this aligns with the PR's goal to add "PostHog analytics across multiple apps."
packages/shared/src/constants.ts (1)
35-36: LGTM!The PostHog cookie name constant is well-defined and will be consistently used across all apps for cross-subdomain tracking.
.github/workflows/deploy-preview.yml (4)
212-213: LGTM!PostHog environment variables are correctly sourced from secrets and properly propagated to the Vercel deployment for the web app.
Also applies to: 228-229
284-285: LGTM!PostHog configuration follows the same consistent pattern as other deployments.
Also applies to: 295-296
364-365: LGTM!Admin deployment correctly includes PostHog environment variables with consistent configuration.
Also applies to: 377-378
431-432: LGTM!Docs deployment correctly includes PostHog environment variables. The minimal set of environment variables is appropriate for a documentation site.
Also applies to: 437-438
apps/admin/src/env.ts (1)
24-25: LGTM!PostHog environment variables are properly defined with appropriate Zod validation. The URL validation for
NEXT_PUBLIC_POSTHOG_HOSTensures type safety.Also applies to: 35-36
apps/marketing/src/env.ts (1)
20-21: LGTM!PostHog environment configuration is consistent with the admin app implementation and properly validated.
Also applies to: 30-31
apps/admin/src/instrumentation-client.ts (1)
9-9: No action needed—defaultsis a valid PostHog initialization parameter. The defaults is a date, such as 2025-11-30, for a configuration snapshot used as defaults to initialize PostHog. This default is overridden when you explicitly set a value for any of the options. The configuration in line 9 follows the documented pattern correctly.
| "@superset/shared": "workspace:*", | ||
| "@superset/ui": "workspace:*", | ||
| "@t3-oss/env-nextjs": "^0.13.8", | ||
| "dotenv": "^17.2.3", | ||
| "framer-motion": "^12.23.24", | ||
| "geist": "^1.5.1", | ||
| "next": "^16.0.10", | ||
| "nextra": "^4.6.1", | ||
| "nextra-theme-blog": "^4.6.1", | ||
| "nextra-theme-docs": "^4.6.1", | ||
| "posthog-js": "^1.306.1", | ||
| "react": "^19.2.3", | ||
| "react-dom": "^19.2.3", | ||
| "three": "^0.181.2" | ||
| "three": "^0.181.2", | ||
| "zod": "^4.1.13" |
There was a problem hiding this comment.
Critical security issue: posthog-js affected by Shai-Hulud 2.0 supply chain attack.
The compromised posthog-js package versions contained a pre-install script that ran automatically during installation. PostHog revoked compromised tokens, removed the malicious package versions, and began issuing "known-good" releases. Upgrade posthog-js to version 1.306.2 or later, which are post-incident clean releases.
Regarding other dependencies:
- zod 4.1.13: No known security issues
- @t3-oss/env-nextjs 0.13.8: Compatible with Zod 4; no security concerns
- dotenv 17.2.3: No security advisories identified
- @superset/shared (workspace): Internal dependency; appropriate
Verify that all PostHog packages across the monorepo use versions published after November 24, 2025.
🤖 Prompt for AI Agents
In apps/docs/package.json around lines 15 to 29, the posthog-js version is
pinned to a compromised release; update the "posthog-js" dependency to
">=1.306.2" (e.g. 1.306.2) to pick up the post-incident clean release,
regenerate the lockfile (npm/yarn/pnpm install) and commit the updated lockfile,
and then scan the monorepo for any other PostHog packages and ensure they are
using versions published after 2025-11-24, updating them as needed and
re-installing to remove any pre-install malicious artifacts.
1 participant
Summary
persistence: "cookie"andpersistence_namePostHogUserIdentifiercomponent to identify users to PostHog after authproxy.tsdocumentation note to AGENTS.mdChanges
PostHogUserIdentifieruses Clerk (web apps) or tRPC (desktop) internally to get user dataNEXT_PUBLIC_COOKIE_DOMAINtolocalhostfor development🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.