Conversation
prastoin
force-pushed
the
standard-tooling-agents-and-role-targets
branch
from
7bd2d2d to
4c6ca5e
Compare
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:4545 This environment will automatically shut down when the PR is closed or after 5 hours. |
prastoin
force-pushed
the
standard-tooling-agents-and-role-targets
branch
2 times, most recently
from
a3739db to
a4b72f7
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
packages/twenty-server/src/engine/workspace-manager/dev-seeder/services/dev-seeder.service.ts
Fixed
Show fixed
Hide fixed
prastoin
force-pushed
the
standard-tooling-agents-and-role-targets
branch
2 times, most recently
from
59ca4d7 to
bc328d4
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@cubic-dev-ai please review |
@prastoin I have started the AI code review. It will take a few minutes to complete. |
![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 63 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-server/src/engine/workspace-manager/twenty-standard-application/utils/field-metadata/create-standard-relation-field-flat-metadata.util.ts">
<violation number="1" location="packages/twenty-server/src/engine/workspace-manager/twenty-standard-application/utils/field-metadata/create-standard-relation-field-flat-metadata.util.ts:29">
P1: The `type` property is added to the context interface but is never destructured or used. The return object hardcodes `type: FieldMetadataType.RELATION`, which means MORPH_RELATION fields will be incorrectly typed. You should destructure `type` from context and use it in the return object.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
...andard-application/utils/field-metadata/create-standard-relation-field-flat-metadata.util.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
2 issues found across 63 files
Prompt for AI agents (all 2 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-server/src/engine/workspace-manager/twenty-standard-application/constants/standard-agent.contant.ts">
<violation number="1" location="packages/twenty-server/src/engine/workspace-manager/twenty-standard-application/constants/standard-agent.contant.ts:1">
P2: Filename typo: `standard-agent.contant.ts` should be `standard-agent.constant.ts` to match the naming convention of other files in the same directory (`standard-object.constant.ts`, `standard-role.constant.ts`, etc.).</violation>
</file>
<file name="packages/twenty-server/src/engine/workspace-manager/twenty-standard-application/utils/field-metadata/create-standard-relation-field-flat-metadata.util.ts">
<violation number="1" location="packages/twenty-server/src/engine/workspace-manager/twenty-standard-application/utils/field-metadata/create-standard-relation-field-flat-metadata.util.ts:29">
P1: The `type` property was added to the context type definition but is not being used. The function still hardcodes `type: FieldMetadataType.RELATION` in the return object, which means MORPH_RELATION fields will have incorrect type metadata. The `type` should be destructured from context and used in the return value.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
...rc/engine/workspace-manager/twenty-standard-application/constants/standard-agent.constant.ts
Show resolved
Hide resolved
...andard-application/utils/field-metadata/create-standard-relation-field-flat-metadata.util.ts
Show resolved
Hide resolved
martmull
left a comment
martmull
left a comment
There was a problem hiding this comment.
I don't really understand why we remove existing agents. Would love an small explanation but overall LGTM
Hey there's an explanation in the PR description :p |
prastoin
force-pushed
the
standard-tooling-agents-and-role-targets
branch
from
b33c519 to
221b7d0
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
📊 API Changes ReportGraphQL Schema ChangesGraphQL Schema Changes[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-schema-introspection.json: Not valid JSON content GraphQL Metadata Schema ChangesGraphQL Metadata Schema Changes[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-metadata-schema-introspection.json: Not valid JSON content REST API Analysis ErrorError OutputREST Metadata API Analysis ErrorError Output
|
| } | ||
|
|
||
| const existingCreateFieldAction = | ||
| createdFieldActionByObjectMetadataId[ | ||
| createFieldAction.objectMetadataId | ||
| ]; | ||
|
|
||
| if (isDefined(existingCreateFieldAction)) { | ||
| return { | ||
| createdFieldActionByObjectMetadataId: { |
There was a problem hiding this comment.
Bug: When aggregating field actions, one conditional branch fails to separate morph/relation fields from other fields, unlike other branches in the same function, causing incorrect aggregation.
Severity: MEDIUM | Confidence: High
🔍 Detailed Analysis
In aggregate-orchestrator-actions-report-create-object-and-create-field-actions.util.ts, when an existingCreateFieldAction is present without an existingCreateObjectAction, the logic incorrectly aggregates all flatFieldMetadatas together. This contradicts the logic in three other branches of the same function, which explicitly separate morphOrRelationFlatFieldMetadatas from otherFlatFieldMetadatas. This logical inconsistency causes non-morph/non-relation fields to be routed to the wrong action type, which could lead to incorrect database schema migrations or migration failures during workspace operations.
💡 Suggested Fix
Update the logic in the conditional branch (lines 154-171) to correctly separate morphOrRelationFlatFieldMetadatas from otherFlatFieldMetadatas. This will align its behavior with the other three conditional branches in the function, ensuring field metadata is routed consistently.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
packages/twenty-server/src/engine/workspace-manager/workspace-migration-v2/utils/aggregate-orchestrator-actions-report-create-object-and-create-field-actions.util.ts#L152-L156
Potential issue: In
`aggregate-orchestrator-actions-report-create-object-and-create-field-actions.util.ts`,
when an `existingCreateFieldAction` is present without an `existingCreateObjectAction`,
the logic incorrectly aggregates all `flatFieldMetadatas` together. This contradicts the
logic in three other branches of the same function, which explicitly separate
`morphOrRelationFlatFieldMetadatas` from `otherFlatFieldMetadatas`. This logical
inconsistency causes non-morph/non-relation fields to be routed to the wrong action
type, which could lead to incorrect database schema migrations or migration failures
during workspace operations.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7585088
# Introduction Caught red handed, introduced a failing command in #16499 that was failing even in system build which is should not
# Introduction Caught red handed, introduced a failing command in #16499 that was failing even in system build which is should not
3 participants
Introduction
In this pullrequest have been migrated to the flat standard entities: Role, Agent and RoleTargets.
What happens
createMorphOrRelationStandardField