Several project config improvements #811
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis set of changes introduces a comprehensive refactor and extension of the configuration schema, validation, and normalization logic across the backend and shared packages. It includes stricter schema enforcement, improved type safety, new migration and sanitization utilities, expanded TypeScript type helpers, and enhanced schema introspection. Several new utility modules and functions are added for currency and date handling, and documentation is updated to clarify the config system. Minor adjustments and cleanups are made in various backend scripts and handlers to align with the new config logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Route
participant Config_Lib
participant Schema_Utils
Client->>API_Route: PATCH /internal/email-templates/[templateId]
API_Route->>Config_Lib: overrideEnvironmentConfigOverride({ tsxSource, themeId? })
Config_Lib->>Schema_Utils: sanitizeEnvironmentConfig
Schema_Utils-->>Config_Lib: Sanitized config
Config_Lib-->>API_Route: Save override if valid
API_Route-->>Client: Response
sequenceDiagram
participant Backend
participant Config_Lib
participant Schema_Utils
Backend->>Config_Lib: validateEnvironmentConfigOverride(override)
Config_Lib->>Schema_Utils: getConfigOverrideErrors
Schema_Utils-->>Config_Lib: Result<null, string>
Config_Lib-->>Backend: Validation result
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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). (9)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Greptile Summary
This PR implements a comprehensive refactor of Stack Auth's configuration management system. The changes introduce several key improvements:
Configuration System Overhaul: The core configuration system has been completely refactored with enhanced validation, migration support, and sanitization. The new system provides better error categorization (FORMAT ERROR, ERROR, WARNING) and includes comprehensive test coverage. Configuration rendering now includes normalization and sanitization steps, with migration support for backwards compatibility.
Type Safety Improvements: Significant enhancements to TypeScript utility types have been added, including better handling of union/intersection types, object manipulation utilities, and more precise type assertions. The configuration schemas now use more sophisticated typing with OptionalKeys and RequiredKeys to distinguish between required and optional fields.
Domain Storage Refactor: The trusted domains configuration has been changed from an array structure to a Map-like object structure where each domain gets a unique UUID as a key. This improves indexing, lookup performance, and follows the codebase pattern for dynamic key configurations.
Enhanced Schema Infrastructure: New schema introspection capabilities have been added through hasNested and enhanced getNested methods, along with a metadata system (stackSchemaInfo) that tracks schema types for better validation and error reporting.
Payment System Foundation: New currency handling utilities and money amount schemas have been introduced, suggesting preparation for billing/subscription features with Stripe integration.
Bug Fixes: Several critical issues were addressed, including a security vulnerability in the development key override logic and proper indentation fixes in the database seed script for GitHub OAuth account creation.
Developer Experience: Various improvements including file watching for migration imports, better error messages with deindent formatting, and more consistent import organization across the codebase.
These changes represent a significant architectural improvement that maintains the same public API while substantially enhancing the internal implementation for better reliability, type safety, and maintainability.
Confidence score: 3/5
• This PR introduces substantial architectural changes that require careful testing, particularly around the configuration validation and migration systems.
• The score reflects concerns about the complexity of the new configuration system, potential runtime issues with the date interval mutations, and incomplete currency handling implementation.
• Files needing attention: packages/stack-shared/src/utils/dates.tsx (mutation issues), packages/stack-shared/src/utils/currencies.tsx (incomplete implementation), and apps/backend/src/lib/config.tsx (complex validation logic).
21 files reviewed, 9 comments
|
😱 Found 3 issues. Time to roll up your sleeves! 😱 🗒️ View all ignored comments in this repo
Need help? Join our Discord for support! |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (11)
apps/backend/prisma/seed.ts (1)
169-169: OAuth variable naming convention violation persists.The 'oauthAuthMethod' variable name still doesn't specify the OAuth flow type context, making it unclear which OAuth flow this auth method belongs to.
apps/backend/package.json (1)
21-21: Update process name list for new migration-imports watcher.The concurrently command needs the process name list updated to include the new migration-imports watcher process.
packages/stack-shared/src/utils/strings.tsx (1)
11-11: Remove test type before merging.This test type is not needed in production code and should be removed.
apps/backend/scripts/verify-data-integrity.ts (2)
78-78: Consider consistent naming convention for CLI flag variables.The variable name
shouldSkipNeonuses camelCase but represents a CLI flag that follows kebab-case (--skip-neon). For consistency with CLI interface parameters, consider using snake_case.- const shouldSkipNeon = flags.includes("--skip-neon"); + const skip_neon = flags.includes("--skip-neon");
135-137: Improve string matching robustness for Neon project detection.The current case-sensitive string matching using
includes("Neon")is error-prone and can lead to false positives. Consider using more robust pattern matching.- if (shouldSkipNeon && projects[i].description.includes("Neon")) { + if (shouldSkipNeon && /\bNeon\b/i.test(projects[i].description)) { return; }packages/stack-shared/src/utils/dates.tsx (1)
148-191: Address the Date mutation issue.This function mutates the input Date object directly, which can cause side effects. A previous review already identified this issue and provided a solution.
The function should create a new Date object to avoid mutating the input:
function applyInterval(date: Date, times: number, interval: Interval): Date { + const result = new Date(date); if (!intervalSchema.isValidSync(interval)) { throw new StackAssertionError(`Invalid interval`, { interval }); } const [amount, unit] = interval; switch (unit) { case 'millisecond': { - date.setMilliseconds(date.getMilliseconds() + amount * times); + result.setMilliseconds(result.getMilliseconds() + amount * times); break; } // ... apply similar changes to all other cases } - return date; + return result; }packages/stack-shared/src/utils/currencies.tsx (1)
51-58: Fix the conversion logic for amounts with missing or incomplete decimals.The current implementation simply removes the decimal point, which won't correctly handle amounts like "10" or "10.5" for currencies with 2 decimal places. These should be converted to 1000 and 1050 respectively, not 10 and 105.
Consider implementing proper decimal handling:
export function moneyAmountToStripeUnits(amount: MoneyAmount, currency: Currency): number { const validated = moneyAmountSchema(currency).defined().validateSync(amount); if (currency.stripeDecimals !== currency.decimals) { - throw new StackAssertionError("unimplemented"); + throw new StackAssertionError("unimplemented: currencies with different Stripe decimals are not yet supported"); } - return Number.parseInt(validated.replace('.', ''), 10); + const [whole, decimal = ''] = validated.split('.'); + const paddedDecimal = decimal.padEnd(currency.decimals, '0'); + return Number.parseInt(whole + paddedDecimal, 10); }packages/stack-shared/src/config/format.ts (2)
152-152: Fix the comment to accurately describe the option.The comment should describe handling null/undefined values, not non-object values.
/** - * What to do if a dot notation is used on a value that is not an object. + * What to do if a dot notation is used on null or undefined values. *
209-212: Add missing break statement to prevent fall-through.The
empty-objectcase is missing a break statement, causing unintended fall-through to the next case.case "empty-object": { set(current, keySegment, {}); + break; }packages/stack-shared/src/schema-fields.ts (1)
373-373: Remove or utilize the unusedidNameparameter.The
idNameparameter is not used in the function body. Either use it for validation/error messages or remove it.-export const userSpecifiedIdSchema = (idName: `${string}Id`) => yupString().matches(/^[a-zA-Z_][a-zA-Z0-9_-]*$/); +export const userSpecifiedIdSchema = (idName: `${string}Id`) => yupString().matches(/^[a-zA-Z_][a-zA-Z0-9_-]*$/, `${idName} must start with a letter or underscore and contain only alphanumeric characters, underscores, and hyphens`);apps/backend/src/lib/config.tsx (1)
400-401: Remove unused variables.These unused variables
daandsrshould be removed.- const da = ((c: any) => c) as any; - const sr = ((c: any) => c) as any; -
🧹 Nitpick comments (5)
packages/stack-shared/src/config/README.md (1)
20-24: Comprehensive documentation of new config features.The updated explanations accurately describe the enhanced config system, particularly the new "config override override" concept and sanitization process.
Consider slightly rephrasing line 23 for conciseness as suggested by static analysis:
-This is most often used eg. in the REST API to let users make changes to the branch-level config, without overwriting the entire branch-level config override. +This is commonly used in the REST API to allow partial updates to branch-level config without overwriting the entire override.packages/stack-shared/src/utils/types.tsx (1)
121-183: Consider documenting the necessity ofanycasts in type assertion functions.While the implementation is sophisticated and the
anycasts appear necessary due to the conditional return types, consider adding a comment explaining why these casts are unavoidable in this type-level programming context.export function typeAssert<T>(): ( IsAny<T> extends true ? TypeAssertionError<`Type assertion failed. Expected true, but got any.`> : IsNever<T> extends true ? TypeAssertionError<`Type assertion failed. Expected true, but got never.`> : T extends true ? (() => undefined) : TypeAssertionError<`Type assertion failed. Expected true, but got: ${TypeToString<T>}`> ) { + // The `any` cast is necessary here because the return type varies based on the input type T return (() => undefined) as any; }apps/backend/src/lib/config.tsx (2)
283-292: Simplify redundant async wrapper.The async wrapper
async (config) => await configis redundant.function getIncompleteProjectConfigQuery(options: ProjectOptions): RawQuery<Promise<ProjectIncompleteConfig>> { - return RawQuery.then( - makeUnsanitizedIncompleteConfigQuery({ - override: getProjectConfigOverrideQuery(options), - schema: projectConfigSchema, - extraInfo: options, - }), - async (config) => await config, - ); + return makeUnsanitizedIncompleteConfigQuery({ + override: getProjectConfigOverrideQuery(options), + schema: projectConfigSchema, + extraInfo: options, + }); }
294-304: Simplify redundant async wrapper.function getIncompleteBranchConfigQuery(options: BranchOptions): RawQuery<Promise<BranchIncompleteConfig>> { - return RawQuery.then( - makeUnsanitizedIncompleteConfigQuery({ - previous: getIncompleteProjectConfigQuery(options), - override: getBranchConfigOverrideQuery(options), - schema: branchConfigSchema, - extraInfo: options, - }), - async (config) => await config, - ); + return makeUnsanitizedIncompleteConfigQuery({ + previous: getIncompleteProjectConfigQuery(options), + override: getBranchConfigOverrideQuery(options), + schema: branchConfigSchema, + extraInfo: options, + }); }packages/stack-shared/src/config/schema.ts (1)
1-2: Make TODO actionable or remove it.The self-deprecating TODO doesn't provide value. Either refactor the code to address the concerns or remove the comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.vscode/settings.json(3 hunks)apps/backend/package.json(1 hunks)apps/backend/prisma/seed.ts(1 hunks)apps/backend/scripts/verify-data-integrity.ts(3 hunks)apps/backend/src/app/api/latest/emails/render-email/route.tsx(1 hunks)apps/backend/src/app/api/latest/internal/email-templates/[templateId]/route.tsx(2 hunks)apps/backend/src/app/api/latest/internal/email-templates/route.tsx(2 hunks)apps/backend/src/app/api/latest/internal/email-themes/route.tsx(2 hunks)apps/backend/src/lib/config.tsx(15 hunks)apps/backend/src/lib/permissions.tsx(5 hunks)apps/backend/src/lib/projects.tsx(1 hunks)apps/backend/src/route-handlers/smart-request.tsx(1 hunks)packages/stack-shared/src/config/README.md(2 hunks)packages/stack-shared/src/config/format.ts(7 hunks)packages/stack-shared/src/config/schema.ts(4 hunks)packages/stack-shared/src/helpers/emails.ts(1 hunks)packages/stack-shared/src/schema-fields.ts(10 hunks)packages/stack-shared/src/utils/currencies.tsx(1 hunks)packages/stack-shared/src/utils/dates.tsx(2 hunks)packages/stack-shared/src/utils/objects.tsx(6 hunks)packages/stack-shared/src/utils/strings.tsx(1 hunks)packages/stack-shared/src/utils/types.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript with strict types, prefertypeoverinterface
Avoid casting toany; Prefer making changes to the API so thatanycasts are unnecessary to access a property or method
Files:
apps/backend/src/app/api/latest/emails/render-email/route.tsxapps/backend/prisma/seed.tsapps/backend/src/app/api/latest/internal/email-templates/route.tsxapps/backend/src/route-handlers/smart-request.tsxapps/backend/src/lib/projects.tsxpackages/stack-shared/src/helpers/emails.tspackages/stack-shared/src/utils/strings.tsxapps/backend/src/lib/permissions.tsxapps/backend/scripts/verify-data-integrity.tspackages/stack-shared/src/utils/dates.tsxapps/backend/src/app/api/latest/internal/email-themes/route.tsxapps/backend/src/app/api/latest/internal/email-templates/[templateId]/route.tsxpackages/stack-shared/src/config/format.tspackages/stack-shared/src/utils/currencies.tsxpackages/stack-shared/src/utils/objects.tsxpackages/stack-shared/src/utils/types.tsxpackages/stack-shared/src/config/schema.tsapps/backend/src/lib/config.tsxpackages/stack-shared/src/schema-fields.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: 2-space indentation, spaces in braces, semicolons required
Return promises withreturn await, no floating promises
Proper error handling for async code with try/catch
Use helper functions:yupXyz()for validation,getPublicEnvVar()for env
Switch cases must use blocks
Files:
apps/backend/src/app/api/latest/emails/render-email/route.tsxapps/backend/prisma/seed.tsapps/backend/src/app/api/latest/internal/email-templates/route.tsxapps/backend/src/route-handlers/smart-request.tsxapps/backend/src/lib/projects.tsxpackages/stack-shared/src/helpers/emails.tspackages/stack-shared/src/utils/strings.tsxapps/backend/src/lib/permissions.tsxapps/backend/scripts/verify-data-integrity.tspackages/stack-shared/src/utils/dates.tsxapps/backend/src/app/api/latest/internal/email-themes/route.tsxapps/backend/src/app/api/latest/internal/email-templates/[templateId]/route.tsxpackages/stack-shared/src/config/format.tspackages/stack-shared/src/utils/currencies.tsxpackages/stack-shared/src/utils/objects.tsxpackages/stack-shared/src/utils/types.tsxpackages/stack-shared/src/config/schema.tsapps/backend/src/lib/config.tsxpackages/stack-shared/src/schema-fields.ts
**/*.{jsx,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{jsx,tsx}: React Server Components preferred where applicable
No direct 'use' imports from React (use React.use instead)
Files:
apps/backend/src/app/api/latest/emails/render-email/route.tsxapps/backend/src/app/api/latest/internal/email-templates/route.tsxapps/backend/src/route-handlers/smart-request.tsxapps/backend/src/lib/projects.tsxpackages/stack-shared/src/utils/strings.tsxapps/backend/src/lib/permissions.tsxpackages/stack-shared/src/utils/dates.tsxapps/backend/src/app/api/latest/internal/email-themes/route.tsxapps/backend/src/app/api/latest/internal/email-templates/[templateId]/route.tsxpackages/stack-shared/src/utils/currencies.tsxpackages/stack-shared/src/utils/objects.tsxpackages/stack-shared/src/utils/types.tsxapps/backend/src/lib/config.tsx
🧠 Learnings (16)
apps/backend/src/app/api/latest/emails/render-email/route.tsx (2)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Proper error handling for async code with try/catch
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to apps/e2e/**/*.test.{ts,tsx} : Import test utilities from /apps/e2e/test/helpers.ts
apps/backend/src/app/api/latest/internal/email-templates/route.tsx (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
apps/backend/src/route-handlers/smart-request.tsx (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
packages/stack-shared/src/helpers/emails.ts (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: packages/stack is generated and will not be committed into the repository; change the files in packages/template instead.
packages/stack-shared/src/utils/strings.tsx (2)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : TypeScript with strict types, prefer type over interface
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Prefer inline snapshot testing with expect(response).toMatchInlineSnapshot(...)
apps/backend/package.json (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
apps/backend/src/lib/permissions.tsx (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
apps/backend/src/app/api/latest/internal/email-themes/route.tsx (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
apps/backend/src/app/api/latest/internal/email-templates/[templateId]/route.tsx (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
packages/stack-shared/src/config/README.md (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: packages/stack is generated and will not be committed into the repository; change the files in packages/template instead.
packages/stack-shared/src/config/format.ts (2)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Switch cases must use blocks
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
packages/stack-shared/src/utils/objects.tsx (2)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : TypeScript with strict types, prefer type over interface
packages/stack-shared/src/utils/types.tsx (2)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : TypeScript with strict types, prefer type over interface
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
packages/stack-shared/src/config/schema.ts (2)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
apps/backend/src/lib/config.tsx (3)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Return promises with return await, no floating promises
packages/stack-shared/src/schema-fields.ts (4)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to apps/e2e/**/*.test.{ts,tsx} : Import test utilities from /apps/e2e/test/helpers.ts
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Proper error handling for async code with try/catch
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
🧬 Code Graph Analysis (6)
apps/backend/src/app/api/latest/internal/email-templates/route.tsx (1)
packages/stack-shared/src/utils/objects.tsx (2)
typedEntries(263-265)filterUndefined(373-375)
apps/backend/src/route-handlers/smart-request.tsx (1)
packages/stack-shared/src/utils/env.tsx (2)
getNodeEnvironment(65-67)getEnvVariable(16-58)
apps/backend/src/lib/projects.tsx (3)
packages/stack-shared/src/utils/objects.tsx (1)
typedFromEntries(281-283)packages/stack-shared/src/utils/uuids.tsx (1)
generateUuid(3-8)packages/stack-shared/src/config/schema.ts (1)
OrganizationRenderedConfig(841-841)
apps/backend/src/app/api/latest/internal/email-themes/route.tsx (1)
packages/stack-shared/src/utils/objects.tsx (2)
typedEntries(263-265)filterUndefined(373-375)
packages/stack-shared/src/utils/types.tsx (2)
packages/stack-shared/src/utils/strings.tsx (1)
Join(5-9)packages/stack-shared/src/utils/objects.tsx (1)
DeepPartial(19-19)
packages/stack-shared/src/schema-fields.ts (5)
packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(67-83)packages/stack-shared/src/utils/strings.tsx (1)
deindent(237-240)packages/stack-shared/src/utils/dates.tsx (2)
Interval(145-145)DayInterval(146-146)packages/stack-shared/src/utils/currencies.tsx (2)
Currency(4-8)MoneyAmount(49-49)packages/stack-shared/src/utils/uuids.tsx (1)
isUuid(25-27)
🪛 LanguageTool
packages/stack-shared/src/config/README.md
[style] ~23-~23: Consider shortening or rephrasing this to strengthen your wording.
Context: ...n used eg. in the REST API to let users make changes to the branch-level config, without overwr...
(MAKE_CHANGES)
⏰ 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). (9)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: setup-tests
- GitHub Check: all-good
- GitHub Check: Security Check
🔇 Additional comments (75)
.vscode/settings.json (1)
27-27: LGTM! Spell checker dictionary updates align with codebase changes.The added words appropriately support the new terminology introduced in this PR's configuration and migration improvements.
Also applies to: 32-32, 82-82, 97-97, 104-104
apps/backend/src/app/api/latest/emails/render-email/route.tsx (1)
5-5: LGTM! Import additions align with enhanced error handling.The additional imports for
StackAssertionErrorandcaptureErrorsupport the broader error handling improvements across the codebase mentioned in the PR objectives.apps/backend/src/route-handlers/smart-request.tsx (1)
263-263: LGTM! Security enhancement correctly implemented.The inverted logic properly tightens security by requiring the explicit dangerous override flag when allowing development key overrides in non-development/test environments. The condition correctly validates that both the environment check fails AND the override flag is not set before blocking the operation.
apps/backend/scripts/verify-data-integrity.ts (1)
118-118: LGTM! Database query updated correctly.Adding the
descriptionfield to the Prisma select is necessary for the new filtering logic.apps/backend/src/app/api/latest/internal/email-templates/route.tsx (2)
1-7: LGTM! Import organization improved.The addition of
filterUndefinedandtypedEntriesutilities enhances type safety and data cleanliness, aligning with similar improvements across internal API routes.
32-37: Excellent type safety and data consistency improvements.Using
typedEntriesprovides better typing thanObject.entriesby excluding numeric keys, andfilterUndefinedensures clean API responses by removing undefined values. This approach is consistent with the enhanced configuration handling patterns across the codebase.apps/backend/src/lib/projects.tsx (1)
168-176: Excellent schema normalization for domains configuration.The transformation from array-based to keyed object structure using
typedFromEntriesand UUID keys improves configuration management and aligns with the broader schema normalization effort. This change supports better configuration migration and validation as referenced in the schema updates.packages/stack-shared/src/helpers/emails.ts (1)
104-104: LGTM! Structural enhancement for theme management.Adding explicit
"themeId": undefinedto all default email templates provides a consistent structure that supports the refined theme management system. This change enhances flexibility while maintaining backward compatibility and aligns with the broader configuration system improvements across the codebase.Also applies to: 109-109, 114-114, 119-119, 124-124
apps/backend/src/app/api/latest/internal/email-themes/route.tsx (2)
4-6: LGTM! Improved type safety with utility functions.The addition of
typedEntriesandfilterUndefinedimports aligns with the broader codebase refactoring to use type-safe utility functions.
75-78: Confirm intent of removed auto-theme-setting logicThe switch to
typedEntriesand use offilterUndefinedimproves type safety and keeps the response clean. However, it looks like the previous logic that automatically set a default or “active” theme whenselectedThemeIdwas missing has been removed from the GET handler in
apps/backend/src/app/api/latest/internal/email-themes/route.tsx. That behavior may have been relied on by clients expecting an active theme fallback.Please confirm that dropping the auto-setting of an active theme here is intentional and that no consumers depend on it.
– File: apps/backend/src/app/api/latest/internal/email-themes/route.tsx
– Lines: ~70–80 (aroundcurrentActiveThemeand the themes mapping)apps/backend/src/app/api/latest/internal/email-templates/[templateId]/route.tsx (2)
2-7: LGTM! Better import organization.The import reordering improves logical grouping and adds necessary schema validation for the updated template handling.
62-64: DisplayName override removal is correctThis
PATCHhandler is scoped to updating the template’s TSX source and theme only—template display names aren’t managed here (they’re updated via a separate endpoint). RemovingdisplayNamefrom this override is intentional and safe. No further changes needed.packages/stack-shared/src/config/README.md (1)
2-3: Helpful accessibility improvement.Adding the ChatGPT suggestion with appropriate disclaimer makes the complex technical documentation more accessible while being transparent about AI limitations.
apps/backend/src/lib/permissions.tsx (4)
175-175: LGTM! Simplified due to improved type guarantees.Removing the fallback defaults reflects the enhanced config system's stricter typing and normalization, ensuring
containedPermissionIdsis always defined.
213-227: Improved config override handling with centralized validation.Using
overrideEnvironmentConfigOverrideensures consistent validation, sanitization, and migration of configuration changes, improving safety over direct database updates.
272-299: Enhanced safety for complex permission updates.The centralized override handling ensures that complex permission relationship updates are properly validated and sanitized, maintaining data integrity.
347-364: Consistent centralized handling for permission deletion.Using the helper function ensures that permission deletion and reference cleanup are properly validated and sanitized through the centralized pipeline.
packages/stack-shared/src/utils/dates.tsx (2)
145-146: Well-structured interval type definitions.The
IntervalandDayIntervaltypes provide clear, type-safe representations for date arithmetic operations with appropriate granularity levels.
193-199: Clean and intuitive API design.The
addIntervalandsubtractIntervalfunctions provide a user-friendly interface for common date operations, with clear semantics and simple implementation.packages/stack-shared/src/utils/currencies.tsx (3)
4-8: LGTM!The Currency type is well-defined with appropriate use of
Uppercase<string>for type-safe currency codes.
10-47: LGTM!The supported currencies are correctly defined with appropriate decimal configurations. JPY correctly has 0 decimals as it doesn't use fractional units.
49-49: LGTM!The
MoneyAmounttype correctly uses template literals to represent monetary values as strings, avoiding floating-point precision issues.packages/stack-shared/src/config/format.ts (2)
5-5: LGTM!Good use of the new
OptionalKeysandRequiredKeysutilities to improve type precision in_NormalizesTo.Also applies to: 18-24
169-183: LGTM!The
isNormalizedandassertNormalizedfunctions correctly verify the normalized state of configs.packages/stack-shared/src/utils/objects.tsx (3)
19-22: LGTM!Excellent improvements to the deep type utilities. The array handling in
DeepPartialand the newDeepRequiredOrUndefinedandDeepOmittypes provide powerful type transformations.Also applies to: 261-262
263-265: LGTM!Good improvement to exclude numeric keys from
typedEntriesandtypedKeys, providing better type safety for objects.Also applies to: 304-306
399-408: LGTM!The
deepFilterUndefinedimplementation correctly usesisObjectLikefor recursive filtering, and the type assertion helps validate the type transformation.packages/stack-shared/src/utils/types.tsx (3)
7-17: LGTM!Sophisticated implementation of
IsUnionandLastUnionElementwith proper handling of edge cases likeneverandany.
44-56: LGTM!Well-implemented key extraction utilities with clever use of conditional types and Pick to determine optional vs required keys.
91-112: LGTM!Impressive implementation of
TypeToStringthat recursively converts types to human-readable string representations. Excellent for debugging and error messages.packages/stack-shared/src/schema-fields.ts (5)
23-49: LGTM!Well-designed schema interface extensions with comprehensive metadata support for various schema types.
63-82: LGTM!The
hasNestedimplementation correctly handles different schema types with proper path validation and error handling.
83-98: LGTM!The
getNestedimplementation correctly retrieves nested schemas with proper handling of union types.
185-253: LGTM!Good improvements to schema constructors with metadata support and better validation for unknown properties.
365-384: LGTM!Excellent implementation of interval and money amount schemas with comprehensive validation rules.
apps/backend/src/lib/config.tsx (18)
2-3: Import updates look good.The imports correctly bring in all the necessary functions and types for the refactored configuration handling.
5-5: Correct use of yup helper functions.Following the coding guidelines to use
yupXyz()helper functions for validation.
8-8: Appropriate utility imports.The additional object utility functions are used throughout the refactored code.
10-10: String utility imports added correctly.The imports support string operations needed in the refactored code.
64-70: Clean refactor to unified validation.The function now properly delegates to the centralized
validateConfigOverrideSchemafunction.
75-81: Proper base config retrieval for validation.The function correctly retrieves the project config as the base for branch validation.
135-135: Good migration on retrieval.Migrating config overrides on retrieval ensures backward compatibility with older configs.
150-150: Consistent migration pattern.
169-169: Proper null handling in migration.The
?? {}ensures safe migration even when no config exists.
184-184: Consistent migration implementation.
195-197: Helpful clarifying comments.The comments properly document the caller's responsibilities for validation.
212-212: Essential validation before persistence.The assertion ensures no invalid configs are saved to the database.
227-227: Improved type safety.
248-248: Consistent validation pattern.
272-272: Consistent type safety.
330-343: Well-structured helper function.The function properly centralizes the logic for creating incomplete config queries with validation.
345-359: Clean validation delegation.The simplified validation approach with proper delegation is well-structured.
361-385: Comprehensive validation implementation.The function properly validates format, checks for errors, and handles warnings.
packages/stack-shared/src/config/schema.ts (22)
4-4: Appropriate email configuration imports.
6-6: Correct use of yup helper functions.Following coding guidelines for validation helpers.
7-13: Well-organized utility imports.
19-25: Proper yup module augmentation.Good use of TypeScript's module augmentation to add custom schema metadata.
36-53: Well-structured project config schema.The schema properly defines different source of truth variants with required fields.
56-79: Comprehensive RBAC schema.Well-designed permission structure with proper validation patterns.
82-88: Clear API keys configuration.
91-113: Complete authentication schema.Covers all authentication methods with proper structure.
115-118: Simple domain configuration.
119-119: Proper immutability enforcement.Good use of
canNoLongerBeOverriddento prevent sourceOfTruth changes at branch level.
137-142: Well-structured email configuration.
190-239: Well-designed migration system.The migration functions are properly documented and handle various transformation scenarios. Good emphasis on their limited purpose.
241-273: Robust property mapping utility.Good implementation with comprehensive test cases.
275-304: Safe property renaming implementation.Good validation to ensure structural consistency.
307-409: Comprehensive and well-structured defaults.Good use of functions for dynamic defaults and explicit undefined values to prevent omissions.
411-423: Sophisticated type-safe defaults system.The complex types ensure type safety when applying defaults.
424-473: Robust defaults application with excellent tests.The function handles complex scenarios including dot notation and function-based defaults with comprehensive test coverage.
519-576: Well-implemented sanitization functions.The functions properly normalize configurations and handle special cases like sourceOfTruth and email themes.
578-730: Thorough config override validation.Comprehensive validation logic that properly checks schema compliance and handles restricted properties.
731-734: Good assertion helper with detailed errors.
735-856: Comprehensive type system with proper assertions.The type definitions and assertions ensure type safety throughout the configuration system.
26-32: Verify removal ofas anycast incanNoLongerBeOverriddenThe
as anycast incanNoLongerBeOverridden(packages/stack-shared/src/config/schema.ts lines 26–32) should be avoided per our guidelines. I wasn’t able to find the generic type definitions forschema.concatandschema.getNested(...).metain the current Yup version. Please verify:
- Whether
concatalready returns aSchema<…>with the correct inferred type.- Whether
metapreserves the original schema type without needing ananycast.- If both methods are properly typed, update the function signature to leverage their generics directly.
- Otherwise, consider adding type overloads or helper types to infer the correct return type and eliminate the
as anycast.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/stack-shared/src/config/schema.ts (1)
26-31: Avoidas anycast in canNoLongerBeOverridden function.The function uses
as anycast which violates coding guidelines. Consider improving type inference to avoid this cast.
🧹 Nitpick comments (1)
packages/stack-shared/src/config/schema.ts (1)
1-1: Consider removing or addressing the TODO comment.The TODO comment suggests code quality concerns. If the code truly needs refactoring, create a proper issue to track this work rather than leaving a TODO comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/backend/package.json(1 hunks)apps/backend/src/lib/config.tsx(15 hunks)apps/backend/src/lib/email-rendering.tsx(1 hunks)packages/stack-shared/src/config/format.ts(7 hunks)packages/stack-shared/src/config/schema.ts(4 hunks)packages/stack-shared/src/utils/strings.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/backend/src/lib/email-rendering.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/stack-shared/src/utils/strings.tsx
- apps/backend/src/lib/config.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript with strict types, prefertypeoverinterface
Avoid casting toany; Prefer making changes to the API so thatanycasts are unnecessary to access a property or method
Files:
packages/stack-shared/src/config/format.tspackages/stack-shared/src/config/schema.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: 2-space indentation, spaces in braces, semicolons required
Return promises withreturn await, no floating promises
Proper error handling for async code with try/catch
Use helper functions:yupXyz()for validation,getPublicEnvVar()for env
Switch cases must use blocks
Files:
packages/stack-shared/src/config/format.tspackages/stack-shared/src/config/schema.ts
🧠 Learnings (3)
packages/stack-shared/src/config/format.ts (3)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Switch cases must use blocks
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
packages/stack-shared/src/config/schema.ts (3)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : TypeScript with strict types, prefer type over interface
apps/backend/package.json (1)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
🧬 Code Graph Analysis (1)
packages/stack-shared/src/config/format.ts (3)
packages/stack-shared/src/utils/types.tsx (2)
OptionalKeys(44-46)RequiredKeys(47-49)packages/stack-shared/src/utils/errors.tsx (1)
StackAssertionError(67-83)packages/stack-shared/src/utils/objects.tsx (4)
set(533-535)get(537-541)hasAndNotUndefined(560-562)deleteKey(564-570)
🪛 Biome (2.1.2)
apps/backend/package.json
[error] 21-21: Missing closing quote
The closing quote must be on the same line.
(parse)
🪛 GitHub Actions: Ensure Prisma migrations are in sync with the schema
apps/backend/package.json
[error] 21-21: pnpm install failed: Bad control character in string literal in JSON at position 1545 (line 21 column 229) in package.json.
🪛 GitHub Actions: Runs E2E API Tests
apps/backend/package.json
[error] 21-21: pnpm install failed: Bad control character in string literal in JSON at position 1545 (line 21 column 229). JSON parsing error in package.json.
🪛 GitHub Actions: Dev Environment Test
apps/backend/package.json
[error] 21-21: pnpm install failed due to JSON parse error: Bad control character in string literal at position 1545 in package.json.
🪛 GitHub Actions: Runs E2E API Tests with external source of truth
apps/backend/package.json
[error] 21-21: pnpm install failed due to JSON parse error: Bad control character in string literal at position 1545.
🪛 GitHub Actions: Run setup tests
apps/backend/package.json
[error] 1-1: pnpm install failed due to JSON parse error: Bad control character in string literal in JSON at position 1545.
🪛 GitHub Actions: Lint & build
apps/backend/package.json
[error] 21-21: pnpm install failed: Bad control character in string literal in JSON at position 1545 (line 21 column 229) while parsing package.json.
⏰ 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: docker
- GitHub Check: all-good
- GitHub Check: Security Check
🔇 Additional comments (7)
packages/stack-shared/src/config/format.ts (7)
5-5: LGTM!The import of
OptionalKeysandRequiredKeysutility types aligns with their usage in the_NormalizesTotype definition and follows the project's pattern of importing specific utility types.
20-21: Improved type precision with utility types.The use of
OptionalKeysandRequiredKeysprovides better type safety by correctly distinguishing between optional and required properties in the normalized config type. The conditional handling for required keys that can be undefined is also correct.
169-183: Well-implemented utility functions for normalization checking.The
isNormalizedandassertNormalizedfunctions correctly identify normalized configs by checking for the absence of dot notation keys and null values. The assertion function follows TypeScript best practices with proper type narrowing.
145-160: Well-designed normalization options with clear documentation.The expanded
NormalizeOptionsprovides granular control over dot notation handling. The separation ofonDotIntoNonObjectandonDotIntoNulloptions allows for flexible configuration normalization strategies. The documentation clearly explains each option's behavior.
202-225: Correct implementation of dot notation handling logic.The normalization logic properly handles the different options for dot notation behavior:
- The "like-non-object" fallback correctly delegates to
onDotIntoNonObjectbehavior- The "empty-object" case properly sets an empty object with a break statement
- All switch cases use blocks as required by coding guidelines
- Error messages are descriptive and include relevant context
228-228: Correct propagation of normalization options.The options parameter is properly threaded through all helper functions (
normalizeValue,setNormalizedValue) to ensure consistent normalization behavior throughout the process.Also applies to: 233-233, 236-236, 240-240, 246-246
266-266: Comprehensive test coverage for new normalization options.The test updates properly validate:
- The
onDotIntoNonObject: "ignore"behavior in the main test case- Error scenarios with updated parameter names
- The new test case specifically covering dot notation into non-objects with "ignore" option
The expected results correctly match the documented behavior.
Also applies to: 280-280, 284-284, 287-287, 294-298
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/stack-shared/src/config/schema.ts (1)
490-517: Avoid multipleas anycasts.The functions use multiple
as anycasts which violate coding guidelines. This issue was previously identified and should be addressed by improving type inference.
🧹 Nitpick comments (1)
packages/stack-shared/src/config/schema.ts (1)
1-1: Consider addressing the TODO comment.The comment suggests code quality concerns. If this file has become complex due to accumulated changes, consider refactoring it into smaller, more focused modules.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/backend/package.json(1 hunks)packages/stack-shared/src/config/schema.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/backend/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript with strict types, prefertypeoverinterface
Avoid casting toany; Prefer making changes to the API so thatanycasts are unnecessary to access a property or method
Files:
packages/stack-shared/src/config/schema.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: 2-space indentation, spaces in braces, semicolons required
Return promises withreturn await, no floating promises
Proper error handling for async code with try/catch
Use helper functions:yupXyz()for validation,getPublicEnvVar()for env
Switch cases must use blocks
Files:
packages/stack-shared/src/config/schema.ts
🧠 Learnings (1)
packages/stack-shared/src/config/schema.ts (3)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : TypeScript with strict types, prefer type over interface
⏰ 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). (5)
- GitHub Check: docker
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: all-good
- GitHub Check: Security Check
🔇 Additional comments (6)
packages/stack-shared/src/config/schema.ts (6)
36-187: LGTM! Well-structured schema definitions.The schema definitions are comprehensive and well-organized. The removal of optional wrappers and enforcement of required keys for RBAC, API keys, auth, domains, and emails configurations improves schema strictness as intended.
190-304: LGTM! Excellent migration system implementation.The migration functions are well-designed with:
- Clear documentation about purpose and constraints
- Proper handling of different config levels
- Comprehensive test coverage for helper functions
- Logical migration steps for historical schema changes
The separation of concerns and the emphasis on reproducibility are particularly good practices.
312-489: LGTM! Comprehensive default configuration system.The default configuration objects are well-structured with explicit undefined values, and the applyDefaults function with dot notation support is excellently implemented with comprehensive test coverage.
520-578: LGTM! Well-implemented sanitization functions.The sanitization functions properly handle:
- Config level hierarchy with appropriate delegation
- SourceOfTruth normalization based on type
- Email theme/template merging with defaults
- Proper validation using assertNormalized
The implementation follows good patterns and maintains type safety.
580-784: LGTM! Comprehensive validation system.The validation functions implement sophisticated schema validation with:
- Proper error handling using Result types
- Advanced schema introspection and restriction
- Appropriate handling of deprecated properties
- Comprehensive validation for all schema types
The complexity is well-justified by the thorough validation requirements.
787-856: LGTM! Excellent TypeScript type system.The type definitions are well-structured with:
- Clear naming conventions for different config stages (override, incomplete, rendered)
- Proper use of advanced TypeScript utility types
- Comprehensive type assertions for validation
- Appropriate config level hierarchy
The type system provides excellent type safety and follows TypeScript best practices.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/stack-shared/src/config/schema.ts (2)
26-31: Avoidas anycast in canNoLongerBeOverridden function.
500-527: Avoid multipleas anycasts.
🧹 Nitpick comments (1)
packages/stack-shared/src/config/schema.ts (1)
1-1: Consider proper refactoring instead of the self-deprecating comment.While the humor is appreciated, this TODO comment is unprofessional. Consider breaking down this file into smaller, focused modules (e.g.,
schema-definitions.ts,migration-utils.ts,validation-utils.ts, etc.) to improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/stack-shared/src/config/schema.ts(4 hunks)packages/stack-shared/src/utils/objects.tsx(7 hunks)turbo.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- turbo.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stack-shared/src/utils/objects.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: TypeScript with strict types, prefertypeoverinterface
Avoid casting toany; Prefer making changes to the API so thatanycasts are unnecessary to access a property or method
Files:
packages/stack-shared/src/config/schema.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: 2-space indentation, spaces in braces, semicolons required
Return promises withreturn await, no floating promises
Proper error handling for async code with try/catch
Use helper functions:yupXyz()for validation,getPublicEnvVar()for env
Switch cases must use blocks
Files:
packages/stack-shared/src/config/schema.ts
🧠 Learnings (1)
packages/stack-shared/src/config/schema.ts (3)
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use helper functions: yupXyz() for validation, getPublicEnvVar() for env
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : Avoid casting to any; Prefer making changes to the API so that any casts are unnecessary to access a property or method
Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-28T23:25:51.701Z
Learning: Applies to **/*.{ts,tsx} : TypeScript with strict types, prefer type over interface
⏰ 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). (9)
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: all-good
- GitHub Check: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: Security Check
🔇 Additional comments (6)
packages/stack-shared/src/config/schema.ts (6)
57-59: Clarify the intent of.isTrue().optional()pattern.The combination of
.isTrue().optional()is confusing. If a field is optional, it typically means it can beundefined. However, constraining it totruewhen present seems to indicate you want eithertrueorundefined, neverfalse. Consider using.oneOf([true])or documenting why this pattern is necessary.Also applies to: 69-69, 70-70
190-305: Well-structured migration functions with clear documentation.The migration functions are well-implemented with:
- Clear documentation about their purpose (database migrations only)
- Comprehensive test coverage
- Proper handling of nested properties with dot notation
- Good separation of concerns between different migration operations
307-484: Excellent implementation of defaults system with strong type safety.The defaults implementation demonstrates:
- Clever use of TypeScript's type system to ensure type safety
- Support for function-based defaults for dynamic key generation
- Comprehensive test coverage for edge cases including dot notation
- Clear separation between different config levels
530-588: Well-implemented sanitization functions with proper type safety.The sanitization functions correctly:
- Assert normalization before processing
- Handle sourceOfTruth type narrowing with proper type guards
- Merge default email themes/templates appropriately
- Maintain type safety throughout the chain
590-794: Comprehensive validation system with clear error/warning separation.The validation implementation excels in:
- Clear distinction between blocking errors and non-blocking warnings
- Thorough schema introspection with proper handling of nested types
- Good error messages that guide users
- Proper handling of the
canNoLongerBeOverriddenmetadata- Smart decision to prefer records over arrays for config structures
796-866: Excellent type definitions with comprehensive compile-time checks.The type system implementation:
- Provides clear types for each stage of config processing
- Uses
Expandfor better IDE tooltips and debugging- Includes compile-time assertions to catch type errors early
- Maintains clear separation between override, incomplete, and rendered configs
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances the project configuration system with significant TypeScript utilities, improved validation mechanisms, and comprehensive support for currency/date operations. The changes modernize the config schema system with stricter type safety, better error handling, and granular control over configuration validation at different organizational levels.
- Enhanced TypeScript utilities: Added sophisticated type manipulation helpers, config schema introspection, and validation utilities in
types.tsxandschema-fields.ts - Improved configuration system: Refactored config override logic with migration support, sanitization functions, and stricter schema enforcement across project/branch/environment/organization levels
- New domain-specific utilities: Added currency conversion support and date interval arithmetic functionality
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Added new environment variables for Google AI, Discord integration |
| packages/stack-shared/src/utils/types.tsx | Major expansion with advanced TypeScript type utilities and assertion helpers |
| packages/stack-shared/src/utils/strings.tsx | Added typed string joining utilities with compile-time type safety |
| packages/stack-shared/src/utils/objects.tsx | Enhanced object manipulation utilities and improved type definitions |
| packages/stack-shared/src/utils/dates.tsx | New date interval arithmetic functions with schema validation |
| packages/stack-shared/src/utils/currencies.tsx | New currency handling utilities with Stripe integration support |
| packages/stack-shared/src/schema-fields.ts | Extensive schema system improvements with better introspection and validation |
| packages/stack-shared/src/helpers/emails.ts | Updated email template structure to include theme ID fields |
| packages/stack-shared/src/config/schema.ts | Massive refactor of config schema system with migration, validation, and sanitization |
| packages/stack-shared/src/config/format.ts | Enhanced config normalization with better error handling options |
| packages/stack-shared/src/config/README.md | Updated documentation with clearer technical explanations |
| docs/templates/overview.mdx | Fixed broken navigation links |
| apps/backend/src/route-handlers/smart-request.tsx | Fixed logic error in development key override validation |
| apps/backend/src/lib/projects.tsx | Updated trusted domains structure to use unique IDs |
| apps/backend/src/lib/permissions.tsx | Refactored to use new config override helpers |
| apps/backend/src/lib/email-rendering.tsx | Removed debug logging and cleaned imports |
| apps/backend/src/lib/config.tsx | Major refactor of config validation and override logic |
| apps/backend/src/app/api/latest/internal/email-themes/route.tsx | Simplified theme handling by removing automatic updates |
| apps/backend/src/app/api/latest/internal/email-templates/route.tsx | Updated to use new typed entry functions |
| apps/backend/src/app/api/latest/internal/email-templates/[templateId]/route.tsx | Simplified template update logic |
| apps/backend/src/app/api/latest/emails/render-email/route.tsx | Added error handling imports |
| apps/backend/scripts/verify-data-integrity.ts | Added flag to skip Neon projects in verification |
| apps/backend/prisma/seed.ts | Fixed missing brace in seeding logic |
| apps/backend/package.json | Added migration imports watcher to development workflow |
| .vscode/settings.json | Updated spell check dictionary |
Important
This PR enhances project config with advanced TypeScript utilities, improved validation, and support for currency conversions, while fixing bugs and refactoring for better schema enforcement.
This description was created by
for 68ea382. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
Chores