Conversation
Greptile OverviewGreptile SummaryThis PR extends the existing NestJS GraphQL patch to significantly reduce schema type leakage between the 'core' and 'metadata' GraphQL schemas. The previous implementation only filtered resolvers by scope, but types could still leak across schemas. This update introduces a comprehensive type reachability analysis using breadth-first search (BFS) to compute which types are actually reachable from each schema's resolvers. Key improvements:
Technical details:
This ensures that when Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Schema as GraphQLSchemaFactory
participant Generator as TypeDefinitionsGenerator
participant Storage as TypeDefinitionsStorage
participant Registry as OrphanedReferenceRegistry
participant TMS as TypeMetadataStorage
Schema->>Generator: clearTypeDefinitionStorage()
Generator->>Storage: clear()
Storage->>Storage: Reset all Maps & Links
Schema->>Registry: clear()
Registry->>Registry: Clear registry
Schema->>Schema: computeReachableTypes(resolvers, orphanedTypes)
Schema->>TMS: getQueriesMetadata()
Schema->>TMS: getMutationsMetadata()
Schema->>TMS: getSubscriptionsMetadata()
Schema->>Schema: Filter by resolvers scope
Schema->>Schema: Seed queue with return types & args
Schema->>Schema: BFS traversal
loop Process Queue
Schema->>TMS: Get metadata for type
Schema->>Schema: Enqueue property types
Schema->>Schema: Enqueue interfaces
end
Schema->>TMS: getObjectTypesMetadata()
Schema->>Schema: Include interface implementations
Schema->>TMS: getUnionsMetadata()
Schema->>Schema: Include all union members
Schema->>Generator: generate(options, reachableTypes)
Generator->>TMS: Filter metadata by reachableTypes
Generator->>Storage: Add filtered type definitions
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-server/patches/@nestjs+graphql+12.1.1.patch">
<violation number="1" location="packages/twenty-server/patches/@nestjs+graphql+12.1.1.patch:173">
P2: Enum types are not filtered by `reachableTypes`, unlike all other type kinds (objects, inputs, interfaces, unions). This means enum types will still leak across GraphQL schemas, undermining the goal of this patch. `generateEnumDefs` should accept `reachableTypes` and filter accordingly, and `computeReachableTypes` should track reachable enums (e.g., enums referenced by field types).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| - this.generateUnionDefs(); | ||
| + generate(options, reachableTypes) { | ||
| + this.generateUnionDefs(reachableTypes); | ||
| this.generateEnumDefs(); |
There was a problem hiding this comment.
P2: Enum types are not filtered by reachableTypes, unlike all other type kinds (objects, inputs, interfaces, unions). This means enum types will still leak across GraphQL schemas, undermining the goal of this patch. generateEnumDefs should accept reachableTypes and filter accordingly, and computeReachableTypes should track reachable enums (e.g., enums referenced by field types).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-server/patches/@nestjs+graphql+12.1.1.patch, line 173:
<comment>Enum types are not filtered by `reachableTypes`, unlike all other type kinds (objects, inputs, interfaces, unions). This means enum types will still leak across GraphQL schemas, undermining the goal of this patch. `generateEnumDefs` should accept `reachableTypes` and filter accordingly, and `computeReachableTypes` should track reachable enums (e.g., enums referenced by field types).</comment>
<file context>
@@ -14,53 +14,230 @@ index 1234567..abcdefg 100644
+- this.generateUnionDefs();
++ generate(options, reachableTypes) {
++ this.generateUnionDefs(reachableTypes);
+ this.generateEnumDefs();
+- this.generateInterfaceDefs(options);
+- this.generateObjectTypeDefs(options);
</file context>
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:21733 This environment will automatically shut down when the PR is closed or after 5 hours. |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".yarn/patches/@nestjs-graphql-npm-12.1.1-96ceb55a68.patch">
<violation number="1" location=".yarn/patches/@nestjs-graphql-npm-12.1.1-96ceb55a68.patch:200">
P2: Enum type definitions are not filtered by `reachableTypes`, unlike all other type generators (input, object, interface, union). This means enum types will leak across schemas. `generateEnumDefs` should accept `reachableTypes` and filter accordingly, and `computeReachableTypes` should track reachable enum references during the BFS traversal.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
📊 API Changes ReportGraphQL Schema ChangesGraphQL Schema Changes[log] [log] ✖ Type AISystemPromptPreview was removed GraphQL Metadata Schema ChangesGraphQL Metadata Schema Changes[log] [log] ✖ Type AppTokenEdge was removed
|
|
Hey @charlesBochet! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
2 participants
Reduce type leakage between GraphQL schemas
Why
Twenty runs two separate GraphQL schemas: core and metadata. NestJS's
@nestjs/graphqluses a globalTypeMetadataStoragethat accumulates all decorated types across all modules. When each schema is built, every registered type leaks into both schemas regardless of which module it belongs to.This means the core schema's generated TypeScript (
generated/graphql.ts) contained ~2,700 lines of types that only belong to the metadata schema (and vice versa). This creates confusion about type ownership, inflates generated code, and makes it harder to reason about which API surface each schema actually exposes.How
1. Patch
@nestjs/graphqlto support schema-scoped type resolutionresolverSchemaScopeoption toGqlModuleOptions, allowing each schema to declare a scope (e.g.'metadata')ResolversExplorerServicenow filters resolvers by aRESOLVER_SCHEMA_SCOPEmetadata key, so each schema only sees its own resolversGraphQLSchemaFactorynow performs a reachability walk (computeReachableTypes) starting from scoped resolver return types and arguments, only including types that are transitively referenced — handling unions, interfaces, and prototype chains2. Register
ClientConfigas orphaned type in metadata schemaSince
ClientConfigis needed in the metadata schema but not directly returned by a resolver, it's explicitly declared viabuildSchemaOptions.orphanedTypes.3. Regenerate frontend types and fix imports
generated/graphql.tsshrank by ~2,700 lines (types moved to where they belong)generated-metadata/graphql.tsgained types likeClientConfigthat were previously missing