Conversation
…ers/crud.tsx Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…te.tsx Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…improve config management in StackAdminInterface
apps/backend/src/app/api/latest/internal/tenancy/current/crud.tsx
Outdated
Show resolved
Hide resolved
![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/backend/src/app/api/latest/internal/tenancy/current/crud.tsx (1)
17-26: Update handler ignores input data - potential issue.The
onUpdatehandler acceptsdataparameter but doesn't use it to perform any actual updates. It returns the same static tenancy information as the read handler, which seems incorrect for an update operation.Consider either:
- Implementing actual update logic using the
dataparameter- Removing the update handler if tenancy updates aren't supported
- Adding a comment explaining why updates are no-ops
onUpdate: async ({ auth, data }) => { + // TODO: Implement actual tenancy update logic or remove if not supported return { id: auth.tenancy.id, project_id: auth.project.id, branch_id: auth.tenancy.branchId, organization_id: auth.tenancy.organization?.id, config: auth.tenancy.completeConfig, }; },
🧹 Nitpick comments (1)
apps/e2e/tests/js/config.test.ts (1)
30-49: Inconsistent property access and unclear update syntax.The test uses bracket notation (
config['auth']) instead of dot notation, and the update syntax with dot notation in string keys ('auth.allowSignUp': false) may not work as expected depending on theupdateConfigimplementation.Consider using consistent property access:
- expect(config['auth']).toMatchInlineSnapshot(` + expect(config.auth).toMatchInlineSnapshot(`Also verify that the update syntax with dot notation in keys is supported by the backend:
await project.updateConfig({ - 'auth.allowSignUp': false, + auth: { allowSignUp: false }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
apps/backend/prisma/seed.ts(4 hunks)apps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsx(2 hunks)apps/backend/src/app/api/latest/integrations/neon/oauth-providers/crud.tsx(4 hunks)apps/backend/src/app/api/latest/integrations/neon/projects/provision/route.tsx(2 hunks)apps/backend/src/app/api/latest/internal/config/crud.tsx(1 hunks)apps/backend/src/app/api/latest/internal/config/override/crud.tsx(1 hunks)apps/backend/src/app/api/latest/internal/config/override/route.tsx(1 hunks)apps/backend/src/app/api/latest/internal/config/route.tsx(1 hunks)apps/backend/src/app/api/latest/internal/projects/crud.tsx(2 hunks)apps/backend/src/app/api/latest/internal/projects/current/crud.tsx(2 hunks)apps/backend/src/app/api/latest/internal/tenancy/current/crud.tsx(1 hunks)apps/backend/src/app/api/latest/teams/crud.tsx(4 hunks)apps/backend/src/app/api/latest/users/crud.tsx(7 hunks)apps/backend/src/lib/projects.tsx(1 hunks)apps/backend/src/s3.tsx(1 hunks)apps/e2e/tests/backend/endpoints/api/v1/internal/config.test.ts(1 hunks)apps/e2e/tests/js/config.test.ts(1 hunks)packages/stack-shared/src/interface/admin-interface.ts(2 hunks)packages/stack-shared/src/interface/crud/config.ts(1 hunks)packages/stack-shared/src/interface/crud/tenancy.ts(1 hunks)packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts(5 hunks)packages/template/src/lib/stack-app/projects/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (5)
- apps/backend/src/app/api/latest/internal/config/route.tsx
- apps/backend/src/lib/projects.tsx
- apps/backend/src/app/api/latest/integrations/neon/projects/provision/route.tsx
- apps/backend/src/app/api/latest/internal/config/override/route.tsx
- apps/backend/src/app/api/latest/teams/crud.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/backend/src/app/api/latest/users/crud.tsx
- apps/backend/src/s3.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/internal/projects/current/crud.tsxapps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsxapps/backend/src/app/api/latest/internal/projects/crud.tsxapps/backend/prisma/seed.tsapps/backend/src/app/api/latest/integrations/neon/oauth-providers/crud.tsxapps/backend/src/app/api/latest/internal/tenancy/current/crud.tsxapps/e2e/tests/js/config.test.tspackages/template/src/lib/stack-app/projects/index.tspackages/stack-shared/src/interface/crud/tenancy.tsapps/e2e/tests/backend/endpoints/api/v1/internal/config.test.tsapps/backend/src/app/api/latest/internal/config/crud.tsxapps/backend/src/app/api/latest/internal/config/override/crud.tsxpackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/stack-shared/src/interface/crud/config.tspackages/stack-shared/src/interface/admin-interface.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/internal/projects/current/crud.tsxapps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsxapps/backend/src/app/api/latest/internal/projects/crud.tsxapps/backend/prisma/seed.tsapps/backend/src/app/api/latest/integrations/neon/oauth-providers/crud.tsxapps/backend/src/app/api/latest/internal/tenancy/current/crud.tsxapps/e2e/tests/js/config.test.tspackages/template/src/lib/stack-app/projects/index.tspackages/stack-shared/src/interface/crud/tenancy.tsapps/e2e/tests/backend/endpoints/api/v1/internal/config.test.tsapps/backend/src/app/api/latest/internal/config/crud.tsxapps/backend/src/app/api/latest/internal/config/override/crud.tsxpackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/stack-shared/src/interface/crud/config.tspackages/stack-shared/src/interface/admin-interface.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/internal/projects/current/crud.tsxapps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsxapps/backend/src/app/api/latest/internal/projects/crud.tsxapps/backend/src/app/api/latest/integrations/neon/oauth-providers/crud.tsxapps/backend/src/app/api/latest/internal/tenancy/current/crud.tsxapps/backend/src/app/api/latest/internal/config/crud.tsxapps/backend/src/app/api/latest/internal/config/override/crud.tsx
apps/e2e/**/*.test.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Import test utilities from
/apps/e2e/test/helpers.ts
Files:
apps/e2e/tests/js/config.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/config.test.ts
**/*.test.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Prefer inline snapshot testing with
expect(response).toMatchInlineSnapshot(...)
Files:
apps/e2e/tests/js/config.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/config.test.ts
🧠 Learnings (6)
📚 Learning: applies to **/*.{js,jsx,ts,tsx} : use helper functions: `yupxyz()` for validation, `getpublicenvvar(...
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
Applied to files:
apps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsxapps/backend/prisma/seed.tsapps/e2e/tests/js/config.test.tspackages/template/src/lib/stack-app/projects/index.tsapps/backend/src/app/api/latest/internal/config/override/crud.tsxpackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
📚 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 apps/e2e/**/*.test.{ts,tsx} : Import test utilities from `/apps/e2e/test/helpers.ts`
Applied to files:
apps/e2e/tests/js/config.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/config.test.ts
📚 Learning: applies to **/*.test.{js,jsx,ts,tsx} : prefer inline snapshot testing with `expect(response).tomatch...
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(...)`
Applied to files:
apps/e2e/tests/js/config.test.ts
📚 Learning: `packages/stack` is generated and will not be committed into the repository; change the files in `pa...
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.
Applied to files:
packages/template/src/lib/stack-app/projects/index.tspackages/stack-shared/src/interface/admin-interface.ts
📚 Learning: applies to **/*.{ts,tsx} : avoid casting to `any`; prefer making changes to the api so that `any` ca...
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
Applied to files:
packages/template/src/lib/stack-app/projects/index.ts
📚 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 **/*.{js,jsx,ts,tsx} : Proper error handling for async code with try/catch
Applied to files:
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
🧬 Code Graph Analysis (10)
apps/backend/src/app/api/latest/internal/projects/current/crud.tsx (1)
apps/backend/src/lib/projects.tsx (1)
createOrUpdateProjectWithLegacyConfig(63-283)
apps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsx (1)
apps/backend/src/lib/projects.tsx (1)
createOrUpdateProjectWithLegacyConfig(63-283)
apps/backend/src/app/api/latest/internal/projects/crud.tsx (1)
apps/backend/src/lib/projects.tsx (1)
createOrUpdateProjectWithLegacyConfig(63-283)
apps/backend/prisma/seed.ts (1)
apps/backend/src/lib/projects.tsx (1)
createOrUpdateProjectWithLegacyConfig(63-283)
apps/backend/src/app/api/latest/integrations/neon/oauth-providers/crud.tsx (1)
apps/backend/src/lib/projects.tsx (1)
createOrUpdateProjectWithLegacyConfig(63-283)
apps/backend/src/app/api/latest/internal/tenancy/current/crud.tsx (4)
packages/stack-shared/src/utils/proxies.tsx (1)
createLazyProxy(61-127)apps/backend/src/route-handlers/crud-handler.tsx (1)
createCrudHandlers(99-294)packages/stack-shared/src/interface/crud/tenancy.ts (1)
tenancyCrud(17-32)packages/stack-shared/src/schema-fields.ts (1)
yupObject(245-249)
apps/e2e/tests/js/config.test.ts (3)
apps/e2e/tests/helpers.ts (1)
it(10-10)apps/e2e/tests/js/js-helpers.ts (1)
createApp(37-74)packages/stack-shared/src/utils/objects.tsx (1)
pick(409-411)
packages/template/src/lib/stack-app/projects/index.ts (2)
packages/stack-shared/src/config/schema.ts (2)
OrganizationRenderedConfig(858-858)EnvironmentConfigOverrideOverride(822-822)packages/stack-shared/src/helpers/production-mode.ts (1)
ProductionModeError(5-8)
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts (5)
packages/template/src/lib/stack-app/apps/implementations/common.ts (2)
createCache(22-27)useAsyncCache(145-190)packages/stack-shared/src/interface/crud/config.ts (1)
ConfigCrud(38-38)packages/stack-shared/src/config/schema.ts (2)
OrganizationRenderedConfig(858-858)EnvironmentConfigOverrideOverride(822-822)packages/template/src/lib/stack-app/projects/index.ts (2)
AdminProjectUpdateOptions(40-45)adminProjectUpdateOptionsToCrud(46-96)packages/stack-shared/src/helpers/production-mode.ts (1)
getProductionModeErrors(10-56)
packages/stack-shared/src/interface/admin-interface.ts (1)
packages/stack-shared/src/interface/crud/config.ts (2)
ConfigCrud(38-38)ConfigOverrideCrud(22-22)
⏰ 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: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: all-good
- GitHub Check: restart-dev-and-test
- GitHub Check: Security Check
🔇 Additional comments (36)
apps/backend/src/app/api/latest/internal/projects/current/crud.tsx (2)
2-2: LGTM - Function import updated correctly.The import statement has been properly updated to use the renamed function
createOrUpdateProjectWithLegacyConfig.
20-25: LGTM - Function call updated correctly.The function call has been properly updated to use the renamed function while maintaining all existing parameters and logic.
apps/backend/src/app/api/latest/integrations/custom/projects/provision/route.tsx (2)
2-2: LGTM - Function import updated correctly.The import statement has been properly updated to use the renamed function
createOrUpdateProjectWithLegacyConfig.
31-52: LGTM - Function call updated correctly.The function call has been properly updated to use the renamed function while preserving all existing parameters and configuration logic.
apps/backend/src/app/api/latest/internal/projects/crud.tsx (2)
2-2: LGTM - Function import updated correctly.The import statement has been properly updated to include the renamed function
createOrUpdateProjectWithLegacyConfig.
33-43: LGTM - Function call updated correctly.The function call has been properly updated to use the renamed function while maintaining all existing parameters and project creation logic.
apps/backend/prisma/seed.ts (4)
3-3: LGTM - Function import updated correctly.The import statement has been properly updated to use the renamed function
createOrUpdateProjectWithLegacyConfig.
37-55: LGTM - Function call updated correctly.The function call for creating the internal project has been properly updated to use the renamed function while preserving all existing configuration parameters.
63-80: LGTM - Function call updated correctly.The function call for updating the internal project has been properly updated to use the renamed function while maintaining all existing configuration update logic.
238-254: LGTM - Function call updated correctly.The function call for creating the emulator project has been properly updated to use the renamed function while preserving all existing emulator configuration parameters.
apps/backend/src/app/api/latest/integrations/neon/oauth-providers/crud.tsx (4)
1-1: LGTM - Function import updated correctly.The import statement has been properly updated to use the renamed function
createOrUpdateProjectWithLegacyConfig.
100-117: LGTM - Function call updated correctly.The function call in the
onCreatehandler has been properly updated to use the renamed function while maintaining all existing OAuth provider configuration logic.
127-140: LGTM - Function call updated correctly.The function call in the
onUpdatehandler has been properly updated to use the renamed function while preserving all existing OAuth provider update logic.
156-167: LGTM - Function call updated correctly.The function call in the
onDeletehandler has been properly updated to use the renamed function while maintaining all existing OAuth provider deletion logic.packages/stack-shared/src/interface/crud/tenancy.ts (5)
1-4: Imports look correct and consistent.The imports follow the established pattern for CRUD interface definitions and use the appropriate schema utilities.
5-11: Read schema correctly defines required tenancy fields.The schema properly requires core tenancy identifiers (
project_id,branch_id,id,config) while makingorganization_idoptional, which aligns with the tenancy model.
13-15: Update schema appropriately restricts updatable fields.Only allowing
configupdates in the update schema is a reasonable design choice that prevents modification of core tenancy identifiers.
17-32: CRUD creation and documentation are well-structured.The API documentation provides clear summaries and descriptions for both read and update operations, with appropriate tagging for API organization.
33-33: Type export follows established pattern.The
TenancyCrudtype alias correctly exports the inferred type from the CRUD object.apps/backend/src/app/api/latest/internal/config/crud.tsx (2)
1-5: Imports are correct and follow established patterns.All necessary dependencies are properly imported for creating CRUD handlers with lazy loading.
6-13: Config CRUD handlers implementation is correct.The lazy proxy pattern is appropriate, and the
onReadhandler correctly extracts and stringifies the tenancy configuration from the auth context. The empty params schema is suitable for this endpoint.apps/backend/src/app/api/latest/internal/tenancy/current/crud.tsx (2)
1-5: Imports are correct and consistent.All necessary dependencies are properly imported following the established pattern for CRUD handlers.
6-16: Read handler correctly returns tenancy data.The
onReadhandler properly extracts and returns all required tenancy fields from the auth context, matching the expected schema.packages/stack-shared/src/interface/admin-interface.ts (3)
2-2: Import addition is correct.The import properly adds the required CRUD types for the new configuration methods.
447-454: getConfig method implementation is correct.The method follows the established pattern for GET requests in the admin interface, using proper endpoint and return type.
456-469: updateConfig method implementation is correct.The method properly wraps the config override in a
config_override_stringfield as expected by the backend API, follows the established PATCH request pattern, and uses appropriate return typing.apps/e2e/tests/js/config.test.ts (3)
1-4: Imports follow coding guidelines correctly.Test utilities are imported from the correct location (
../helpers) and other dependencies are properly imported.
5-28: First test correctly validates config retrieval.The test properly creates an admin app, retrieves project config, and uses inline snapshot testing as required by the coding guidelines. The picked config sections provide focused testing.
51-64: Second snapshot correctly validates config update.The test properly verifies that the configuration change is reflected in the updated config, and the snapshot shows the expected change from
truetofalseforallowSignUp.packages/template/src/lib/stack-app/projects/index.ts (1)
4-4: LGTM! Well-structured configuration management methods.The new methods follow the established patterns for async/reactive data access in the codebase, with appropriate platform-specific annotations for the React hook.
Also applies to: 26-29
apps/backend/src/app/api/latest/internal/config/override/crud.tsx (1)
9-49: LGTM! Robust error handling and validation flow.The implementation correctly validates JSON parsing, applies schema validation, and returns the updated configuration. The use of
createLazyProxyfor deferred initialization is a good performance optimization.apps/e2e/tests/backend/endpoints/api/v1/internal/config.test.ts (1)
1-533: LGTM! Comprehensive test coverage with proper inline snapshots.The test suite thoroughly covers all aspects of the config API including access control, CRUD operations, validation, and error handling. Tests follow the prescribed patterns with inline snapshots and proper imports from the helpers file.
packages/stack-shared/src/interface/crud/config.ts (1)
1-38: LGTM! Well-structured CRUD schemas with proper documentation.The schemas correctly use yup validation helpers and provide clear API documentation metadata. The type exports follow TypeScript best practices.
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts (3)
21-21: LGTM! Cache initialization follows established patterns.The config overrides cache is correctly initialized using the same
createCachepattern as other caches in the class.Also applies to: 23-23, 61-63
90-92: LGTM! Simple and effective helper method.The JSON parsing helper follows the naming convention of other CRUD parsing methods in the class.
159-170: LGTM! Config methods properly integrated with cache and interface.The three config methods are well-implemented:
getConfigcorrectly fetches and parses the configurationuseConfigproperly uses the cache with memoization and platform guardsupdateConfigappropriately delegates to the interface methodThe implementation is consistent with other similar methods in the class.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/backend/src/s3.tsx (1)
6-11: Add validation for S3_ENDPOINT to prevent malformed URLs.Empty S3_ENDPOINT will cause malformed URLs in
getS3PublicUrl. Consider adding validation or using a more appropriate default.-const S3_ENDPOINT = getEnvVariable("STACK_S3_ENDPOINT", ""); +const S3_ENDPOINT = getEnvVariable("STACK_S3_ENDPOINT", "").replace(/\/$/, ""); // Remove trailing slash if presentAdditionally, consider adding validation:
+if (S3_ENDPOINT && !S3_ENDPOINT.startsWith("http")) { + throw new StackAssertionError("STACK_S3_ENDPOINT must be a valid URL"); +}
🧹 Nitpick comments (1)
apps/backend/src/s3.tsx (1)
81-101: LGTM on main upload function.The function properly handles all input cases (base64, URL, null, undefined) with appropriate type safety and error handling. The explicit null/undefined differentiation maintains API consistency.
Consider adding JSDoc documentation for better API documentation:
+/** + * Uploads a base64 image to S3 or returns the URL if already a valid URL. + * @param input - Base64 image data, URL, null, or undefined + * @param folderName - S3 folder to store the image in + * @returns Promise resolving to the public URL, null, or undefined + */ export async function uploadAndGetUrl(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/backend/prisma/schema.prisma(2 hunks)apps/backend/src/app/api/latest/team-member-profiles/crud.tsx(1 hunks)apps/backend/src/app/api/latest/teams/crud.tsx(2 hunks)apps/backend/src/app/api/latest/users/crud.tsx(4 hunks)apps/backend/src/s3.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/backend/prisma/schema.prisma
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/backend/src/app/api/latest/users/crud.tsx
- apps/backend/src/app/api/latest/teams/crud.tsx
- apps/backend/src/app/api/latest/team-member-profiles/crud.tsx
🧰 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/s3.tsx
**/*.{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/s3.tsx
**/*.{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/s3.tsx
🧠 Learnings (1)
📚 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 **/*.{js,jsx,ts,tsx} : Proper error handling for async code with try/catch
Applied to files:
apps/backend/src/s3.tsx
⏰ 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: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: all-good
- GitHub Check: lint_and_build (latest)
- GitHub Check: Security Check
🔇 Additional comments (4)
apps/backend/src/s3.tsx (4)
1-4: LGTM on imports and image processing integration.The imports are well-structured and properly utilize the helper functions from the coding guidelines (
getEnvVariable) and integrate with the new image processing module.
12-26: LGTM on S3 client configuration.The conditional client initialization and configuration options are appropriate for S3-compatible services. The
forcePathStyle: trueis particularly important for services like S3Mock used in local development.
32-72: LGTM on image upload implementation.The variable scoping issue from previous reviews has been resolved -
bufferandformatare now properly declared before the try block. The error handling appropriately convertsImageProcessingErrortoStatusError, and the S3 upload logic is sound.
74-79: LGTM on image string validation.The regex pattern correctly handles various image formats including webp with the
[a-zA-Z0-9]+pattern, addressing previous concerns about format matching.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docker/dependencies/docker.compose.yaml (1)
140-143: Host port 8121 already used elsewhere – confirm no collisionA previous review flagged this exact port as occupied by
react-example. Re-check the overall compose stack and dev environment before merging; a clash will make either service unreachable.
🧹 Nitpick comments (3)
docker/dependencies/docker.compose.yaml (2)
138-138: Remove trailing whitespace to satisfy YAMLlintYAMLlint flags extraneous spaces on this otherwise-blank line.
- +
150-151:wgetmay be absent in theadobe/s3mockimageIf the image lacks
wget, the container will enter an unhealthy state.
Consider switching to the busybox‐providedcurlalready present in most slim images, or installwgetexplicitly.- test: ["CMD", "wget", "--quiet", "--tries=1", "--spider", "http://localhost:9090/"] + test: ["CMD", "curl", "-fs", "http://localhost:9090/"]apps/backend/.env.development (1)
51-56: Minor ordering & consistency nitDotenv parsers ignore key order, but keeping related keys grouped improves scanability and silences
dotenv-linter. Suggested block:# S3 Configuration for local development using s3mock STACK_S3_BUCKET=stack-storage STACK_S3_ENDPOINT=http://localhost:8121 STACK_S3_REGION=us-east-1 STACK_S3_ACCESS_KEY_ID=s3mockroot STACK_S3_SECRET_ACCESS_KEY=s3mockrootNo functional change—purely cosmetic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/backend/.env.development(1 hunks)apps/dev-launchpad/public/index.html(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/teams.test.ts(3 hunks)apps/e2e/tests/backend/endpoints/api/v1/users.test.ts(1 hunks)docker/dependencies/docker.compose.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/dev-launchpad/public/index.html
- apps/e2e/tests/backend/endpoints/api/v1/users.test.ts
- apps/e2e/tests/backend/endpoints/api/v1/teams.test.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: `packages/stack` is generated and will not be committed into the repository; change the files in `pa...
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.
Applied to files:
apps/backend/.env.development
🪛 dotenv-linter (3.3.0)
apps/backend/.env.development
[warning] 54-54: [UnorderedKey] The STACK_S3_ACCESS_KEY_ID key should go before the STACK_S3_ENDPOINT key
[warning] 56-56: [UnorderedKey] The STACK_S3_BUCKET key should go before the STACK_S3_ENDPOINT key
🪛 YAMLlint (1.37.1)
docker/dependencies/docker.compose.yaml
[error] 138-138: trailing spaces
(trailing-spaces)
⏰ 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: lint_and_build (latest)
- GitHub Check: docker
- GitHub Check: all-good
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: Security Check
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/backend/src/auto-migrations/auto-migration.tests.ts(1 hunks)
🧰 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:
apps/backend/src/auto-migrations/auto-migration.tests.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/auto-migrations/auto-migration.tests.ts
🧠 Learnings (1)
📚 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 apps/e2e/**/*.test.{ts,tsx} : Import test utilities from `/apps/e2e/test/helpers.ts`
Applied to files:
apps/backend/src/auto-migrations/auto-migration.tests.ts
🪛 Biome (2.1.2)
apps/backend/src/auto-migrations/auto-migration.tests.ts
[error] 225-225: Shouldn't redeclare 'appliedCounts'. Consider to delete it or rename it.
'appliedCounts' is defined here:
(lint/suspicious/noRedeclare)
⏰ 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: restart-dev-and-test
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: setup-tests
- GitHub Check: all-good
- GitHub Check: build (22.x)
- GitHub Check: docker
- GitHub Check: Security Check
Note for deployment: This PR needs to add some env vars <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Adds S3-compatible storage for profile images with S3Mock for local development, updating environment variables, Docker configurations, and tests. > > - **Behavior**: > - Adds S3-compatible storage for user, team, and team member profile images using `uploadAndGetUrl()` in `s3.tsx`. > - Integrates S3Mock for local development in `docker.compose.yaml` and `emulator/docker.compose.yaml`. > - Updates `crud.tsx` files for `users`, `teams`, and `team-member-profiles` to use S3 for profile images. > - **Environment**: > - Adds S3-related environment variables to `.env.development`. > - Updates `package.json` to include `@aws-sdk/client-s3`. > - **Testing**: > - Modifies test cases in `teams.test.ts` and `users.test.ts` to validate S3 URL behavior. > - Updates `auto-migration.tests.ts` for concurrent migration handling. > - **Documentation**: > - Updates `self-host.mdx` to include S3 storage requirements. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=stack-auth%2Fstack-auth&utm_source=github&utm_medium=referral)<sup> for 901f0b5. You can [customize](https://app.ellipsis.dev/stack-auth/settings/summaries) this summary. It will automatically update as commits are pushed.</sup> ---- <!-- ELLIPSIS_HIDDEN --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for uploading and storing user, team, and team member profile images using S3-compatible storage. * Integrated S3Mock for local development and testing of storage features. * Added new services to Docker Compose configurations for S3Mock and related dependencies. * Introduced image validation and processing for base64-encoded images before upload. * **Bug Fixes** * Profile image upload now returns a storage URL instead of echoing back the base64 data. * **Documentation** * Updated self-hosting documentation to include S3 storage requirements and usage. * **Chores** * Added and updated environment variables for S3 storage configuration. * Updated backend dependencies to include the AWS S3 SDK. * Enhanced test cases to validate new image upload and URL behaviors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: Konsti Wohlwend <n2d4xc@gmail.com>
2 participants
Note for deployment: This PR needs to add some env vars
Important
Adds S3-compatible storage for profile images with S3Mock for local development, updating environment variables, Docker configurations, and tests.
uploadAndGetUrl()ins3.tsx.docker.compose.yamlandemulator/docker.compose.yaml.crud.tsxfiles forusers,teams, andteam-member-profilesto use S3 for profile images..env.development.package.jsonto include@aws-sdk/client-s3.teams.test.tsandusers.test.tsto validate S3 URL behavior.auto-migration.tests.tsfor concurrent migration handling.self-host.mdxto include S3 storage requirements.This description was created by
for 901f0b5. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores