Twenty config core implementation #11595

ehconitin · 2025-04-16T06:19:57Z

closes twentyhq/core-team-issues#760

…d add migration file

…implementation

…as the number of env vars in config variables file

…implementation

…fig-core-implementation

greptile-apps

PR Summary

(updates since last review)

Based on the recent changes and previous reviews, I'll focus on the most significant updates to the configuration system:

This PR enhances the configuration system with improved caching and initialization handling.

Added ConfigInitializationState enum to track configuration loading states (NOT_INITIALIZED, INITIALIZING, INITIALIZED, FAILED)
Implemented ConfigSource enum to properly identify configuration origins (ENVIRONMENT, DATABASE, DEFAULT)
Added proper handling of environment-only variables through isEnvOnlyConfigVar utility
Improved type safety with ConfigVariableType and ConfigVariableOptions types
Added basic validation utilities in apply-basic-validators.util.ts for different data types

The changes improve the configuration system's reliability through better state management and type validation while maintaining clear separation between environment and database configurations.

_{33 file(s) reviewed, 10 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-04-23T12:19:24Z

packages/twenty-server/.env.example

@@ -78,4 +78,4 @@ FRONTEND_URL=http://localhost:3001
 # CLOUDFLARE_WEBHOOK_SECRET=
 # IS_CONFIG_VARIABLES_IN_DB_ENABLED=false
 # ANALYTICS_ENABLED=


style: ANALYTICS_ENABLED is empty but should have a default boolean value (true/false)

packages/twenty-server/src/engine/core-modules/twenty-config/cache/config-cache.service.ts

packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts

...erver/src/engine/core-modules/twenty-config/drivers/__tests__/database-config.driver.spec.ts

greptile-apps · 2025-04-23T12:21:10Z

packages/twenty-server/src/engine/core-modules/twenty-config/drivers/database-config.driver.ts

+  @Cron(`0 */${CONFIG_VARIABLES_REFRESH_INTERVAL_MINUTES} * * * *`)
+  async refreshExpiredCache(): Promise<void> {


logic: Cron job could start before initialization is complete. Consider adding initialization check.

greptile-apps · 2025-04-23T12:22:01Z

packages/twenty-server/src/engine/core-modules/twenty-config/storage/config-storage.service.ts

+        this.logger.debug(
+          `Fetching config for ${key as string} in database: ${result?.value}`,
+        );


logic: Logging sensitive configuration values could expose secrets in logs

Suggested change

this.logger.debug(

`Fetching config for ${key as string} in database: ${result?.value}`,

);

this.logger.debug(

`Fetching config for ${key as string}`,

);

greptile-apps · 2025-04-23T12:23:06Z

packages/twenty-server/src/engine/core-modules/twenty-config/utils/config-transformers.util.ts

+    if (typeof value === 'number') {
+      return value !== 0;
+    }


style: Consider handling NaN, Infinity, and -Infinity explicitly in number conversion

greptile-apps · 2025-04-23T12:23:17Z

...ing/message-import-manager/drivers/microsoft/services/microsoft-get-messages.service.spec.ts

+          provide: TwentyConfigService,
+          useValue: {},
+        },


style: Empty mock object for TwentyConfigService may be insufficient. Consider adding necessary mock methods/properties that the service actually uses.

Copilot

Pull Request Overview

This PR implements core configuration functionality along with several refactorings and improvements for the Twenty config system, addressing an issue raised in core-team-issues#760. Key changes include:

Introducing a new enum (ConfigInitializationState) and new interfaces/driver implementations for database- and environment-backed configuration.
Refactoring config metadata decorators and removing legacy cast decorators, with corresponding updates to conversion services and tests.
Adding caching support with a dedicated ConfigCacheService and comprehensive unit tests.

Reviewed Changes

Copilot reviewed 34 out of 35 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/twenty-server/src/engine/core-modules/twenty-config/enums/config-initialization-state.enum.ts	Adds a new enum for representing configuration initialization states.
packages/twenty-server/src/engine/core-modules/twenty-config/drivers/interfaces/database-config-driver.interface.ts	Introduces an interface for database config drivers with new methods.
packages/twenty-server/src/engine/core-modules/twenty-config/drivers/environment-config.driver.ts & database-config.driver.ts	Implements environment and database drivers with logging, error handling, and refresh scheduling.
packages/twenty-server/src/engine/core-modules/twenty-config/decorators/config-variables-metadata.decorator.ts	Updates the metadata decorator to support new options (isEnvOnly, type, options) and applies basic validators.
packages/twenty-server/src/engine/core-modules/twenty-config/conversion/config-value-converter.service.ts & spec	Adds a service for converting values between DB and app representations with unit tests.
packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts	Refactors config variable definitions to use type metadata instead of legacy cast decorators.
packages/twenty-server/src/engine/core-modules/twenty-config/cache/config-cache.service.ts & spec	Implements a cache service for config values with TTL and negative cache support.
packages/twenty-server/src/engine/core-modules/file/*	Cleans up imports by removing unused TwentyConfigService injections.

Files not reviewed (1)

packages/twenty-server/package.json: Language not supported

Comments suppressed due to low confidence (1)

packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts:42

The removal of legacy cast decorators like @CastToBoolean() may lead to unexpected transformation behavior if the new metadata-based validation does not fully replicate the previous behavior. Please verify that the new type metadata and validators cover all transformation scenarios previously handled by the cast decorators.

-  @CastToBoolean()

packages/twenty-server/src/engine/core-modules/twenty-config/drivers/database-config.driver.ts

…implementation

github-actions · 2025-04-24T20:56:39Z

	Warnings
⚠️	Changes were made to the config variables, but not to the documentation - Please review your changes and check if a change needs to be documented!

Generated by 🚫 dangerJS against 275918f

…implementation

github-actions · 2025-04-25T14:25:07Z

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:21544

This environment will automatically shut down when the PR is closed or after 5 hours.

FelixMalfait · 2025-04-25T14:26:07Z

packages/twenty-server/src/engine/core-modules/twenty-config/cache/config-cache.service.ts

+    cacheKeys: string[];
+  } {
+    return {
+      positiveEntries: this.foundConfigValuesCache.size,


Could we get rid of this positive/negative linguo and stick to found/missing you already use

ehconitin added 26 commits April 9, 2025 18:56

change system_var to config_variable and user_var to user_variable an…

e556261

…d add migration file

add IS_CONFIG_VAR_IN_DB_ENABLED

3ad7cb2

Merge remote-tracking branch 'upstream/main' into config-prep-2

1f0a9a6

lint

f61b830

Merge remote-tracking branch 'upstream/main' into config-prep-2

b671d96

rename service spec

e58ef31

Copy over from POC

31ed7e7

continue

46ba298

twenty config module is global?

d5c6038

wip: const renaming

eeb7448

hasty commit, needs rework :)

07744af

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

448cf9a

…implementation

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

2c9748f

…implementation

refactor

7c072cb

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

8a1bee7

…implementation

grep review

3a4c317

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

1ed5e8f

…implementation

dont need this overenginnered code, max entries would always be same …

1eece63

…as the number of env vars in config variables file

WIP: tests - add cache test

be2bf64

WIP: tests -- add tests for drivers

071b147

WIP: tests -- add test for storage service

f0fe2d9

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

5b917f5

…implementation

fix test

c52cecf

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

3f9f84d

…implementation

Merge remote-tracking branch 'upstream/main' into config-prep-2

da53140

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

5f1f1aa

…implementation

ehconitin self-assigned this Apr 16, 2025

ehconitin added -PR: wip -PR: awaiting author labels Apr 16, 2025

Merge remote-tracking branch 'upstream/config-prep-2' into twenty-con…

6646b76

…fig-core-implementation

ehconitin marked this pull request as ready for review April 23, 2025 12:18

ehconitin requested review from FelixMalfait and charlesBochet April 23, 2025 12:18

ehconitin added -PR: awaiting review and removed -PR: wip -PR: awaiting author labels Apr 23, 2025

greptile-apps bot reviewed Apr 23, 2025

View reviewed changes

lint

f0a06ba

ehconitin requested a review from Copilot April 23, 2025 17:24

Copilot AI reviewed Apr 23, 2025

View reviewed changes

packages/twenty-server/src/engine/core-modules/twenty-config/drivers/database-config.driver.ts Outdated Show resolved Hide resolved

ehconitin added 12 commits April 23, 2025 23:20

add debounce mechanism to prevent duplicate config refreshes

ba2a645

lint

2d2de86

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

f58a3ae

…implementation

simplify

c62f677

lint

61aae91

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

50c5565

…implementation

refactor

0393cfc

converters improvements

c8fdaca

add safe guards on conversion

2af3020

add tests and more improvements

9f267e7

lint

883f50a

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

8fef8d1

…implementation

ehconitin force-pushed the twenty-config-core-implementation branch from 2eaee68 to 8fef8d1 Compare April 24, 2025 20:55

FelixMalfait and others added 2 commits April 25, 2025 08:56

Merge branch 'main' into twenty-config-core-implementation

04d8b68

Merge remote-tracking branch 'upstream/main' into twenty-config-core-…

275918f

…implementation

FelixMalfait added the preview-app label Apr 25, 2025

FelixMalfait approved these changes Apr 25, 2025

View reviewed changes

FelixMalfait reviewed Apr 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Twenty config core implementation #11595

Twenty config core implementation #11595

ehconitin commented Apr 16, 2025 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Apr 23, 2025

greptile-apps bot Apr 23, 2025

greptile-apps bot Apr 23, 2025

greptile-apps bot Apr 23, 2025

greptile-apps bot Apr 23, 2025

Copilot AI left a comment

github-actions bot commented Apr 24, 2025 •

edited

Loading

github-actions bot commented Apr 25, 2025 •

edited

Loading

FelixMalfait Apr 25, 2025

		@Cron(`0 /${CONFIG_VARIABLES_REFRESH_INTERVAL_MINUTES} * * *`)
		async refreshExpiredCache(): Promise<void> {

Twenty config core implementation #11595

Are you sure you want to change the base?

Twenty config core implementation #11595

Conversation

ehconitin commented Apr 16, 2025 • edited Loading

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Apr 23, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 23, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 23, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 23, 2025

Choose a reason for hiding this comment

greptile-apps bot Apr 23, 2025

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

github-actions bot commented Apr 24, 2025 • edited Loading

github-actions bot commented Apr 25, 2025 • edited Loading

FelixMalfait Apr 25, 2025

Choose a reason for hiding this comment

ehconitin commented Apr 16, 2025 •

edited

Loading

github-actions bot commented Apr 24, 2025 •

edited

Loading

github-actions bot commented Apr 25, 2025 •

edited

Loading