Skip to content

Fix internal app#17496

Merged
martmull merged 2 commits intomainfrom
fix-internal-app
Jan 27, 2026
Merged

Fix internal app#17496
martmull merged 2 commits intomainfrom
fix-internal-app

Conversation

@martmull
Copy link
Copy Markdown
Contributor

as title

@socket-security
Copy link
Copy Markdown

socket-security bot commented Jan 27, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedtwenty-sdk@​0.0.4 ⏵ 0.3.177 +710098 +2698 +470

View full report

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 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-apps/internal/self-hosting/tsconfig.json">

<violation number="1" location="packages/twenty-apps/internal/self-hosting/tsconfig.json:24">
P2: Add a baseUrl for the new path mappings; TypeScript requires baseUrl when using paths or the config will fail to compile.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

"skipDefaultLibCheck": true,
"resolveJsonModule": true
"resolveJsonModule": true,
"paths": {
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Add a baseUrl for the new path mappings; TypeScript requires baseUrl when using paths or the config will fail to compile.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-apps/internal/self-hosting/tsconfig.json, line 24:

<comment>Add a baseUrl for the new path mappings; TypeScript requires baseUrl when using paths or the config will fail to compile.</comment>

<file context>
@@ -20,7 +20,11 @@
     "skipDefaultLibCheck": true,
-    "resolveJsonModule": true
+    "resolveJsonModule": true,
+    "paths": {
+      "src/*": ["./src/*"],
+      "~/*": ["./*"]
</file context>
Suggested change
"paths": {
"baseUrl": ".",
"paths": {
"src/*": ["./src/*"],
"~/*": ["./*"]
}
Fix with Cubic

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Jan 27, 2026

Greptile Overview

Greptile Summary

Migrated the internal self-hosting app from twenty-sdk 0.0.4 to 0.3.1, updating to the newer SDK API patterns.

Major Changes:

  • Updated SDK from decorator-based APIs (@ObjectMetadata, @FieldMetadata) to function-based APIs (defineApp, defineObject, defineFunction)
  • Restructured files into src/app/ directory following new SDK conventions
  • Added comprehensive npm scripts for app management (dev, sync, auth, etc.)
  • Updated telemetry webhook to use new event type structure with proper ServerlessFunctionEvent typing

Issues Found:

  • String concatenation bug in telemetryWebhook.function.ts that will produce "undefined undefined" when first/last names are missing
  • Changed email field universalIdentifier which may impact existing data - needs verification

Confidence Score: 3/5

  • Safe to merge with fixes - SDK migration is straightforward but has one critical string concatenation bug
  • The SDK migration follows standard patterns and is mostly mechanical, but the string concatenation bug will produce incorrect data ("undefined undefined") for users without first/last names, and the email field UUID change needs verification
  • Pay close attention to packages/twenty-apps/internal/self-hosting/src/app/telemetryWebhook.function.ts (string concatenation bug) and packages/twenty-apps/internal/self-hosting/src/app/selfHostingUser.object.ts (UUID change)

Important Files Changed

Filename Overview
packages/twenty-apps/internal/self-hosting/package.json Updated twenty-sdk from 0.0.4 to 0.3.1 and added npm scripts for app management
packages/twenty-apps/internal/self-hosting/src/app/selfHostingUser.object.ts Replaced decorator-based object definition with defineObject function API, fixed email field UUID
packages/twenty-apps/internal/self-hosting/src/app/telemetryWebhook.function.ts Migrated to defineFunction API and added proper event type definition, potential string concatenation bug with undefined values

Sequence Diagram

sequenceDiagram
    participant Client as Self-hosted Instance
    participant Webhook as Telemetry Webhook Function
    participant TwentyAPI as Twenty GraphQL API
    participant DB as Database (selfHostingUsers)
    
    Client->>Webhook: POST /webhook/telemetry<br/>(user_signup event)
    Note over Webhook: Extract event payload<br/>(email, firstName, lastName)
    
    alt Event is not user_signup
        Webhook-->>Client: {success: true, message: "Event ignored"}
    end
    
    alt No email found
        Webhook-->>Client: {success: false, error: "Missing email"}
    end
    
    alt Email contains "test" or "example"
        Webhook-->>Client: {success: true, message: "Test email ignored"}
    end
    
    Note over Webhook: Create Twenty API client<br/>(with TWENTY_API_KEY)
    
    Webhook->>TwentyAPI: GraphQL mutation<br/>createSelfHostingUser(upsert: true)
    TwentyAPI->>DB: Insert/Update selfHostingUser record
    DB-->>TwentyAPI: Record created/updated
    TwentyAPI-->>Webhook: {id, email}
    
    Webhook-->>Client: {success: true, message: "User created/updated"}
Loading

@martmull martmull enabled auto-merge January 27, 2026 20:42
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Jan 27, 2026

Additional Comments (1)

packages/twenty-apps/internal/self-hosting/src/app/telemetryWebhook.function.ts
String concatenation can produce "undefined undefined" if first/last name are missing

            name:
              [
                payload?.payload?.events?.[0]?.userFirstName,
                payload?.payload?.events?.[0]?.userLastName,
              ]
                .filter(Boolean)
                .join(' ') || null,
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-apps/internal/self-hosting/src/app/telemetryWebhook.function.ts
Line: 99:102

Comment:
String concatenation can produce `"undefined undefined"` if first/last name are missing

```suggestion
            name:
              [
                payload?.payload?.events?.[0]?.userFirstName,
                payload?.payload?.events?.[0]?.userLastName,
              ]
                .filter(Boolean)
                .join(' ') || null,
```

How can I resolve this? If you propose a fix, please make it concise.

@martmull martmull added this pull request to the merge queue Jan 27, 2026
Merged via the queue into main with commit 6b38686 Jan 27, 2026
49 of 51 checks passed
@martmull martmull deleted the fix-internal-app branch January 27, 2026 21:02
@twenty-eng-sync
Copy link
Copy Markdown

Hey @martmull! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

1 similar comment
@twenty-eng-sync
Copy link
Copy Markdown

Hey @martmull! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants