feat: emit metadata events for schema changes with actor context for webhooks#17622
feat: emit metadata events for schema changes with actor context for webhooks#17622mabdullahabaid wants to merge 24 commits intomainfrom
Conversation
Greptile OverviewGreptile SummaryThis PR adds metadata eventing to track schema changes (objectMetadata, fieldMetadata, views, etc.) and emit events for webhooks and future audit logs. It also threads actor context ( Key changes:
Implementation approach:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Resolver
participant Service
participant MigrationService as WorkspaceMigrationValidateBuildAndRunService
participant Runner as WorkspaceMigrationRunnerService
participant EventEmitter as MetadataEventEmitter
participant EventEmitter2 as NestJS EventEmitter2
participant Listener as MetadataEventsToDbListener
participant WebhookQueue as Webhook Message Queue
participant WebhookJob as CallWebhookJobsForMetadataJob
participant WebhookRepo as WebhookRepository
participant CallWebhook as CallWebhookJob
Client->>Resolver: GraphQL Mutation (create/update/delete metadata)
Note over Resolver: @AuthUser() user<br/>@AuthUserWorkspaceId() workspaceMemberId
Resolver->>Service: Pass userId & workspaceMemberId
Service->>MigrationService: validateBuildAndRunWorkspaceMigration({ actorContext })
MigrationService->>MigrationService: Build workspace migration
MigrationService->>Runner: run(workspaceMigration)
Runner-->>MigrationService: Migration complete
MigrationService->>EventEmitter: emitMetadataEventsFromMigration({ actions, actorContext })
EventEmitter->>EventEmitter: groupActionsByMetadataNameAndAction()
loop For each metadata entity & action
EventEmitter->>EventEmitter: emitMetadataBatchEvent({ userId, workspaceMemberId })
EventEmitter->>EventEmitter2: emit('metadata.{entity}.{action}', MetadataEventBatch)
end
EventEmitter2->>Listener: @OnEvent('metadata.*.created/updated/deleted')
Listener->>WebhookQueue: Add CallWebhookJobsForMetadataJob
WebhookQueue->>WebhookJob: Process MetadataEventBatch
WebhookJob->>WebhookRepo: Find matching webhooks
WebhookRepo-->>WebhookJob: Webhook entities
WebhookJob->>WebhookJob: transformMetadataEventBatchToWebhookEvents()
Note over WebhookJob: Includes userId & workspaceMemberId<br/>in webhook payload
loop For each webhook event chunk
WebhookJob->>WebhookQueue: Add CallWebhookJob batch
end
WebhookQueue->>CallWebhook: Process webhook delivery
CallWebhook->>Client: HTTP POST to webhook URL
|
packages/twenty-server/src/engine/metadata-event-emitter/enums/metadata-event-action.enum.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/skill/skill.resolver.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.resolver.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/webhook/webhook.resolver.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/view-field/resolvers/view-field.resolver.ts
Show resolved
Hide resolved
...ages/twenty-server/src/engine/metadata-modules/page-layout/resolvers/page-layout.resolver.ts
Show resolved
Hide resolved
...server/src/engine/metadata-modules/view-filter-group/resolvers/view-filter-group.resolver.ts
Outdated
Show resolved
Hide resolved
...ages/twenty-server/src/engine/metadata-modules/view-filter/resolvers/view-filter.resolver.ts
Show resolved
Hide resolved
Additional Comments (3)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/twenty-server/src/engine/metadata-modules/page-layout-tab/resolvers/page-layout-tab.resolver.ts
Line: 7:8
Comment:
imports not in alphabetical order
`AuthUser` should come before `AuthUserWorkspaceId` alphabetically
```suggestion
import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator';
import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-workspace-id.decorator';
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/twenty-server/src/engine/metadata-modules/page-layout-widget/resolvers/page-layout-widget.resolver.ts
Line: 7:8
Comment:
imports not in alphabetical order
```suggestion
import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator';
import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-workspace-id.decorator';
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: packages/twenty-server/src/engine/metadata-modules/view-group/resolvers/view-group.resolver.ts
Line: 11:12
Comment:
imports not in alphabetical order
```suggestion
import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator';
import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-workspace-id.decorator';
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise. |
packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts
Outdated
Show resolved
Hide resolved
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:43941 This environment will automatically shut down when the PR is closed or after 5 hours. |
packages/twenty-server/src/engine/metadata-modules/webhook/webhook.service.ts
Show resolved
Hide resolved
| @@ -0,0 +1 @@ | |||
| export type MetadataEventAction = 'created' | 'updated' | 'deleted'; | |||
There was a problem hiding this comment.
Should this metadata-event-emitter be in core modules next to event-emitter module?
There was a problem hiding this comment.
I had the same thoughts, but workspace-event-emitter was in the engine too directly while being registered inside core modules, so I followed the convention.
...e-manager/workspace-migration/services/workspace-migration-validate-build-and-run-service.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts
Outdated
Show resolved
Hide resolved
|
Hey there, on prod right now will review afterwards |
There was a problem hiding this comment.
1 issue found across 5 files (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="packages/twenty-front/src/modules/settings/components/SettingsDatabaseEventsForm.tsx">
<violation number="1" location="packages/twenty-front/src/modules/settings/components/SettingsDatabaseEventsForm.tsx:70">
P2: `WebhookEntitySelect` doesn’t actually disable interaction. Even with `disabled={disabled}`, the dropdown can still open and `onChange` will fire, so read-only forms can still modify the entity selection. Update `WebhookEntitySelect` to block clicks/selection when disabled (e.g., pass `disableClickForClickableComponent` to `Dropdown` and guard `handleSelect`).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/twenty-front/src/modules/settings/components/SettingsDatabaseEventsForm.tsx
Show resolved
Hide resolved
prastoin
left a comment
There was a problem hiding this comment.
Hey there ! NIce job overall !
Main concerns:
- We might wanna rethink the introduced type context. We might wanna use the
AuthContext( for apiKey support too ) - Using discriminated union in the twenty-shared type system
packages/twenty-server/src/engine/metadata-event-emitter/types/metadata-event-batch.type.ts
Outdated
Show resolved
Hide resolved
...es/twenty-server/src/engine/metadata-event-emitter/utils/compute-metadata-event-name.util.ts
Outdated
Show resolved
Hide resolved
| import type { AllUniversalWorkspaceMigrationAction } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/workspace-migration-action-common'; | ||
|
|
||
| type MetadataEventActorContext = { | ||
| userId?: string; |
There was a problem hiding this comment.
Question: Could both properties be optional at the same time ? same question with both defined ? ( if not please use one or the other pattern )
There was a problem hiding this comment.
workspaceMemberId is the identifier for workspace‑scoped things like RLS, permissions, and audit trails, while userId is the stable, global identity that helps consumers correlate activity across workspaces or even if a membership is removed/recreated. So for user‑initiated actions that impact metadata, we should set both in my opinion.
Removing workspaceMemberId for now since we can add it later if users request, but I have a feeling that for org-level consumers with multiple workspaces, both can be relevant when integrating with internal systems.
There was a problem hiding this comment.
Not really @mabdullahabaid, for RLS, permissions, etc, we would rather use userWorkspaceId (a third contender!). You can always go from WorkspaceMemberId to userWorkspaceId or to userId, or from userWorkspaceMemberId to WorkspaceMemberId or userId, but you need the workspaceId to go from userId to WorkspaceMemberId or UserWorpsaceId.
In any case, as discussed, I think you should use the existing context object as much as possible rather than re-inventing your own
There was a problem hiding this comment.
@FelixMalfait workspaceId is always emitted in the webhook payload. So a combination of workspaceId and userId should be sufficient to reach userWorkspaceId or workspaceMemberId in that case.
As mentioned, I removed the workspaceMemberId for this reason since a combination of userId and workspaceId helps locate workspaceMemberId and userWorkspaceId when needed.
That works?
packages/twenty-shared/src/metadata-events/metadata-record-create.event.ts
Outdated
Show resolved
Hide resolved
...e-manager/workspace-migration/services/workspace-migration-validate-build-and-run-service.ts
Outdated
Show resolved
Hide resolved
| '*.*', | ||
| ]; | ||
|
|
||
| const webhooks = await this.webhookRepository.find({ |
There was a problem hiding this comment.
Question: cc @charlesBochet do we aim having a dynamic custom webhook flat maps cache entries per operations that could ligthen database impact ?
| private getUpdatedFieldsFromEvent( | ||
| event: MetadataRecordEvent, | ||
| ): string[] | undefined { | ||
| if ('updatedFields' in event.properties) { |
There was a problem hiding this comment.
Nitpick: Not mandatory but using Object.prototype.hasOwnProperty.call is safer and more performant ( especially for the expected volume here )
Same for aboves
|
|
||
| const WEBHOOK_JOBS_CHUNK_SIZE = 20; | ||
|
|
||
| @Processor(MessageQueue.webhookQueue) |
There was a problem hiding this comment.
Praise: Overall very clean !
packages/twenty-shared/src/metadata-events/metadata-record.base.event.ts
Outdated
Show resolved
Hide resolved
| metadataEventBatch: MetadataEventBatch<MetadataRecordEvent>; | ||
| webhooks: WebhookEntity[]; | ||
| }): CallWebhookJobData[] { | ||
| const result: CallWebhookJobData[] = []; |
There was a problem hiding this comment.
Out of scope: We could improve CallWebhookJobData typing by using a discriminated union.
Even the end user as to inspect response integrity in order to determine available fields.
And so our own devxp would be enhanced not having to infer context by properties shape
packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts
Outdated
Show resolved
Hide resolved
| const universalIdentifier = | ||
| 'universalIdentifier' in action ? action.universalIdentifier : undefined; |
There was a problem hiding this comment.
const universalIdentifier =
action.universalIdentifier| action: AllUniversalWorkspaceMigrationAction, | ||
| metadataName: AllMetadataName, | ||
| fromToAllFlatEntityMaps: FromToAllFlatEntityMaps, | ||
| result: GroupedEvents, |
There was a problem hiding this comment.
Remark: not a fan of mutations here
packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts
Outdated
Show resolved
Hide resolved
| fromToAllFlatEntityMaps: FromToAllFlatEntityMaps, | ||
| result: GroupedEvents, | ||
| ): void { | ||
| const fieldMetadatas = |
There was a problem hiding this comment.
Remark: using in should always be as last resort. 99% of the time this could be prevented by improving args typing
packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts
Outdated
Show resolved
Hide resolved
| >; | ||
|
|
||
| @Injectable() | ||
| export class MetadataEventEmitter { |
There was a problem hiding this comment.
Hey @mabdullahabaid lets have a peer session on this file if you feel like to
There was a problem hiding this comment.
1 issue found across 39 files (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="packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts">
<violation number="1" location="packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts:77">
P2: User-initiated metadata events no longer include `workspaceMemberId`. The initiator context contains this field, but the batch event only forwards `userId` and `apiKeyId`, which breaks the stated actor context requirement and removes member attribution for user-authenticated changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-event-emitter/types/metadata-event-batch.type.ts
Show resolved
Hide resolved
...twenty-server/src/engine/metadata-modules/webhook/jobs/call-webhook-jobs-for-metadata.job.ts
Show resolved
Hide resolved
...e-manager/workspace-migration/services/workspace-migration-validate-build-and-run-service.ts
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts
Show resolved
Hide resolved
51831e3 to
f143d7a
Compare
|
Hey @mabdullahabaid I've started to refactor the main emitter service as discussed on discord |
How to get this PR mergedI will try to split the dependency PR in order to unlock this one TL;DR => Lets refactor the |
| this.metadataEventEmitter.emitMetadataEventsFromMigration({ | ||
| actions: validateAndBuildResult.workspaceMigration.actions, | ||
| fromToAllFlatEntityMaps: args.fromToAllFlatEntityMaps, | ||
| workspaceId: args.workspaceId, | ||
| }); |
There was a problem hiding this comment.
Bug: System-level operations like workspace creation trigger metadata events without actor context (userId, apiKeyId) because getWorkspaceAuthContext() fails outside of an HTTP request context.
Severity: MEDIUM
Suggested Fix
Modify the signature of validateBuildAndRunWorkspaceMigrationFromTo to accept an optional initiatorContext. Propagate this context from its callers, such as synchronizeTwentyStandardApplicationOrThrow, and pass it to emitMetadataEventsFromMigration. This ensures system-initiated events can be properly attributed.
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/services/workspace-migration-validate-build-and-run-service.ts#L219-L223
Potential issue: During system-level operations such as workspace initialization or dev
seeding, the `validateBuildAndRunWorkspaceMigrationFromTo` function is called. This
triggers `emitMetadataEventsFromMigration` without an `initiatorContext`. The fallback
mechanism, `getWorkspaceAuthContext()`, fails because these operations run outside of an
HTTP request's async context. A `try-catch` block silently handles this failure,
resulting in metadata events and webhooks being sent without `userId` or `apiKeyId`.
This creates inconsistent audit trails, where user-initiated actions have actor context
but system-initiated ones do not.
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="packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts">
<violation number="1" location="packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts:400">
P2: Index update actions are explicitly ignored, so metadata.index.updated events are never emitted. This creates a gap where index updates won’t trigger webhooks/audit events even though other metadata updates do. Consider emitting an update event using the same entityId-to-universalIdentifier lookup used for other entityId-based metadata actions (view, role, etc.) or provide a concrete custom mapping instead of returning undefined.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| fromToAllFlatEntityMaps: FromToAllFlatEntityMaps; | ||
| }): MetadataUpdateEventWithMetadataName | undefined { | ||
| switch (action.metadataName) { | ||
| case 'index': { |
There was a problem hiding this comment.
P2: Index update actions are explicitly ignored, so metadata.index.updated events are never emitted. This creates a gap where index updates won’t trigger webhooks/audit events even though other metadata updates do. Consider emitting an update event using the same entityId-to-universalIdentifier lookup used for other entityId-based metadata actions (view, role, etc.) or provide a concrete custom mapping instead of returning undefined.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-server/src/engine/metadata-event-emitter/metadata-event-emitter.ts, line 400:
<comment>Index update actions are explicitly ignored, so metadata.index.updated events are never emitted. This creates a gap where index updates won’t trigger webhooks/audit events even though other metadata updates do. Consider emitting an update event using the same entityId-to-universalIdentifier lookup used for other entityId-based metadata actions (view, role, etc.) or provide a concrete custom mapping instead of returning undefined.</comment>
<file context>
@@ -423,6 +397,10 @@ export class MetadataEventEmitter {
fromToAllFlatEntityMaps: FromToAllFlatEntityMaps;
}): MetadataUpdateEventWithMetadataName | undefined {
switch (action.metadataName) {
+ case 'index': {
+ // TODO implement custom index update action transpiler as it's not like the others
+ return undefined
</file context>
|
@prastoin thank you so much for giving a hand with this. I shall wait while taking up some other tasks in the meantime. Please let me know when you're done with the refactor on your end. I shall follow the lead and update my work accordingly. |
Summary
This PR adds metadata eventing: when schema metadata (objectMetadata, fieldMetadata, view, viewField, etc.) is created, updated, or deleted, we now emit events that can trigger webhooks and future audit logs. It also adds actor context (
userId,workspaceMemberId) to those events so subscribers can attribute changes to a user or API key.What changed
1. Metadata eventing (first commit)
New service that emits batch events after successful workspace migrations. Event names follow
metadata.{entity}.{action}(e.g.metadata.objectMetadata.created,metadata.fieldMetadata.updated).Listens for metadata events and enqueues webhook delivery via
CallWebhookJobsForMetadataJob.MetadataEventAction,MetadataEventBatch, and record event types for create/update/delete.Calls the metadata event emitter after running migrations so all metadata changes (from any module) emit events from a single place.
Sourced from the create action payload (
flatEntity/flatFieldMetadatas) becausefromToAllFlatEntityMapsdoes not provide a before/after diff for creates. Update/delete events still use the fromToAllFlatEntityMaps comparison.2. Actor context (second commit)
Accepts optional
actorContext(userId,workspaceMemberId) and includes it on emitted batch events.Passes
actorContextfrom the request into the metadata event emitter.All metadata modules resolve
@AuthUser({ allowUndefined: true })and@AuthUserWorkspaceId()and passuserIdandworkspaceMemberIdthrough to the migration/event pipeline. Both are optional so API-key–authenticated requests (no user) still emit events without a user identity.Shared some questions on Discord about the PR.