feat(desktop): add SKIP_ENV_VALIDATION flag for dev mode#466
feat(desktop): add SKIP_ENV_VALIDATION flag for dev mode#466saddlepaddle merged 1 commit intomainfrom
Conversation
WalkthroughIntroduces a SKIP_ENV_VALIDATION environment flag propagated through Vite build defines and used to skip environment schema validation and the desktop renderer sign-in gating for local/dev runs; adds developer instructions to BUILDING.md and enables the flag in test and bunfig setups. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
Skip env validation and auth screen when SKIP_ENV_VALIDATION=1 is set, useful for local development without credentials. - Add skipValidation to desktop env files and trpc/env.ts - Add SKIP_ENV_VALIDATION to Vite define blocks for proper bundling - Skip auth screen in MainScreen when flag is set - Document usage in BUILDING.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
saddlepaddle
force-pushed
the
feat/skip-env-validation
branch
from
8244a97 to
a1ef5ee
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src/main/env.main.ts (1)
34-34: Verify API compatibility (duplicate concern).Same concern as in packages/trpc/src/env.ts: verify that @t3-oss/env-core v0.13.8 supports the
skipValidationoption.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/index.tsx (1)
35-37: Consider adding NODE_ENV guard for defense in depth.The authentication bypass using
SKIP_ENV_VALIDATIONworks correctly, but could be hardened by also checking thatNODE_ENVis"development". This adds an extra safeguard to prevent accidental authentication bypass in production builds.🔎 Proposed enhancement
const isSignedIn = - !!process.env.SKIP_ENV_VALIDATION || (authState?.isSignedIn ?? false); - const isAuthLoading = !process.env.SKIP_ENV_VALIDATION && !authState; + (!!process.env.SKIP_ENV_VALIDATION && process.env.NODE_ENV === "development") || (authState?.isSignedIn ?? false); + const isAuthLoading = !(!!process.env.SKIP_ENV_VALIDATION && process.env.NODE_ENV === "development") && !authState;apps/desktop/src/renderer/env.renderer.ts (1)
43-45: Consider NODE_ENV guard and document type safety implications.The conditional validation bypass correctly implements the development workflow, but bypasses all runtime type checking when
SKIP_ENV_VALIDATIONis set. This could lead to runtime errors if environment variables are malformed.Consider:
- Adding a
NODE_ENV === "development"check for defense in depth (similar to the auth bypass)- Adding a comment warning that malformed env vars will cause runtime errors when validation is skipped
🔎 Proposed enhancement
+/** + * Skip validation in development mode only. + * WARNING: When validation is skipped, malformed env vars will cause runtime errors. + */ export const env = process.env.SKIP_ENV_VALIDATION + && process.env.NODE_ENV === "development" ? (rawEnv as z.infer<typeof envSchema>) : envSchema.parse(rawEnv);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/BUILDING.mdapps/desktop/electron.vite.config.tsapps/desktop/src/main/env.main.tsapps/desktop/src/renderer/env.renderer.tsapps/desktop/src/renderer/screens/main/index.tsxpackages/trpc/src/env.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/electron.vite.config.tsapps/desktop/src/main/env.main.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/env.renderer.tspackages/trpc/src/env.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/electron.vite.config.tsapps/desktop/src/main/env.main.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/env.renderer.tspackages/trpc/src/env.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: 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.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/electron.vite.config.tsapps/desktop/src/main/env.main.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/env.renderer.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/main/env.main.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/env.renderer.ts
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/env.main.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/env.renderer.ts
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/screens/main/index.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Applied to files:
apps/desktop/electron.vite.config.tsapps/desktop/src/main/env.main.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/env.renderer.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Applied to files:
apps/desktop/src/renderer/env.renderer.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Use Bun as the package manager exclusively - do not use npm, yarn, or pnpm
Applied to files:
apps/desktop/BUILDING.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Run bun run lint to check for lint issues without making changes
Applied to files:
apps/desktop/BUILDING.md
🧬 Code graph analysis (1)
apps/desktop/src/renderer/env.renderer.ts (3)
apps/desktop/src/main/env.main.ts (1)
env(12-38)packages/trpc/src/env.ts (1)
env(4-14)apps/desktop/src/shared/env.shared.ts (1)
env(28-30)
⏰ 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). (6)
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Web
- GitHub Check: Deploy API
- GitHub Check: Build
🔇 Additional comments (3)
apps/desktop/BUILDING.md (1)
1-9: LGTM!The development instructions are clear and correctly document the SKIP_ENV_VALIDATION flag usage. The documentation properly uses Bun as the package manager, consistent with project standards.
apps/desktop/electron.vite.config.ts (1)
61-63: LGTM!The SKIP_ENV_VALIDATION environment variable is correctly propagated across all Vite define blocks (main, preload, and renderer). The pattern
JSON.stringify(process.env.SKIP_ENV_VALIDATION || "")ensures that when the variable is not set, it defaults to an empty string, which is falsy in JavaScript. This means validation will run by default in production builds where the variable is not defined.Also applies to: 110-112, 129-131
packages/trpc/src/env.ts (1)
13-13: @t3-oss/env-core version 0.13.8 supports theskipValidationoption. The skipValidation property is documented in BaseOptions for version 0.13.8 and determines whether to skip validation of environment variables.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/index.tsx (1)
35-37: Auth bypass logic is correct.The logic correctly bypasses authentication when
SKIP_ENV_VALIDATIONis set, allowing development without credentials.Optional: Consider adding a comment for clarity
+ // When SKIP_ENV_VALIDATION is set, bypass auth for local development const isSignedIn = !!process.env.SKIP_ENV_VALIDATION || (authState?.isSignedIn ?? false); const isAuthLoading = !process.env.SKIP_ENV_VALIDATION && !authState;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/BUILDING.mdapps/desktop/bunfig.tomlapps/desktop/electron.vite.config.tsapps/desktop/src/main/env.main.tsapps/desktop/src/renderer/env.renderer.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/test-setup.tspackages/trpc/src/env.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/env.renderer.ts
- apps/desktop/src/main/env.main.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
packages/trpc/src/env.tsapps/desktop/electron.vite.config.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/test-setup.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
packages/trpc/src/env.tsapps/desktop/electron.vite.config.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/test-setup.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: 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.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/electron.vite.config.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/test-setup.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/screens/main/index.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/screens/main/index.tsx
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/screens/main/index.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in src/main/index.ts and electron.vite.config.ts with override: true before other imports
Applied to files:
apps/desktop/electron.vite.config.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/test-setup.ts
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Use Bun as the package manager exclusively - do not use npm, yarn, or pnpm
Applied to files:
apps/desktop/BUILDING.md
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Run bun run lint to check for lint issues without making changes
Applied to files:
apps/desktop/BUILDING.md
⏰ 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). (6)
- GitHub Check: Deploy Web
- GitHub Check: Deploy API
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Marketing
- GitHub Check: Build
🔇 Additional comments (5)
apps/desktop/bunfig.toml (1)
7-7: LGTM!Setting
SKIP_ENV_VALIDATION = "1"in the test environment is appropriate and aligns with the test setup intest-setup.ts.apps/desktop/test-setup.ts (1)
17-17: LGTM!Setting
SKIP_ENV_VALIDATION = "1"in the test setup correctly bypasses environment validation during tests, consistent with the test configuration inbunfig.toml.apps/desktop/electron.vite.config.ts (1)
61-63: LGTM!The
SKIP_ENV_VALIDATIONflag is correctly propagated to all build contexts (main, preload, and renderer) with a safe default of an empty string, ensuring validation runs in production builds when the flag is not explicitly set.Also applies to: 110-112, 129-131
apps/desktop/BUILDING.md (1)
1-9: LGTM!The development instructions are clear and correctly document the
SKIP_ENV_VALIDATION=1 bun run devusage pattern for local development without credentials.packages/trpc/src/env.ts (1)
13-13: No issues found. TheskipValidationoption is supported in @t3-oss/env-core version 0.13.8 and will work as expected.
1 participant
Summary
SKIP_ENV_VALIDATION=1flag to skip env validation and auth screen in dev modeUsage
Test plan
SKIP_ENV_VALIDATION=1 bun run devfromapps/desktop/- should skip auth screenbun run devnormally - should show auth screen as before🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.