Skip to content

Fix hacktoberfest applications#15613

Merged
Weiko merged 14 commits intomainfrom
fix-hacktoberfest-applications
Nov 4, 2025
Merged

Fix hacktoberfest applications#15613
Weiko merged 14 commits intomainfrom
fix-hacktoberfest-applications

Conversation

@martmull
Copy link
Copy Markdown
Contributor

@martmull martmull commented Nov 4, 2025

As title, make them syncable with twenty-cli:0.2.0
image

@socket-security
Copy link
Copy Markdown

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

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedchokidar@​4.0.39910010080100
Addedjsdom@​26.1.0981001009570
Updatedjsonc-parser@​2.2.1 ⏵ 3.3.1100100100 +184100
Addeddotenv@​16.6.110010010090100

View full report

Comment on lines 183 to 185
const options = {
method: 'GET',
headers: {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Calling .find() on undefined when getCompaniesObject() returns {} causes a TypeError.
Severity: HIGH | Confidence: 0.95

🔍 Detailed Analysis

When getCompaniesObject() returns an empty object {}, the subsequent access companyObject?.fields evaluates to undefined. Attempting to call .find() on this undefined value, as seen in companyObject?.fields.find(...), will result in a TypeError: Cannot read property 'find' of undefined. This occurs when the API call succeeds but does not locate a 'company' object, causing getCompaniesObject() to return {} as a fallback.

💡 Suggested Fix

Ensure companyObject is a valid object before accessing companyObject.fields. Check if companyObject is an empty object and handle that case explicitly, or ensure getCompaniesObject consistently returns undefined or a structured object.

🤖 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-apps/hacktoberfest-2025/stripe-synchronizer/serverlessFunctions/stripe/src/index.ts#L166-L185

Potential issue: When `getCompaniesObject()` returns an empty object `{}`, the
subsequent access `companyObject?.fields` evaluates to `undefined`. Attempting to call
`.find()` on this `undefined` value, as seen in `companyObject?.fields.find(...)`, will
result in a `TypeError: Cannot read property 'find' of undefined`. This occurs when the
API call succeeds but does not locate a 'company' object, causing `getCompaniesObject()`
to return `{}` as a fallback.

Did we get this right? 👍 / 👎 to inform future reviews.

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.

Greptile Overview

Greptile Summary

This PR migrates all Hacktoberfest 2025 applications to be compatible with twenty-cli:0.2.0. The migration follows a consistent pattern across all apps:

Key Changes:

  • Removed standalone .manifest.jsonc files for serverless functions
  • Added application.config.ts files at app root to define app metadata and environment variables with universal identifiers
  • Exported config: ServerlessFunctionConfig from serverless function index.ts files, moving trigger definitions from JSON to TypeScript
  • Added tsconfig.json files to standardize TypeScript configuration across apps
  • Updated base CLI template to include allowSyntheticDefaultImports for better module compatibility
  • Applied code formatting improvements (semicolons, spacing, quotes consistency)

Migration Pattern:
Each app now follows the structure: app config defines variables → function code exports trigger configuration → CLI reads both to sync the application.

Issues Found:

  • Critical: Inverted boolean logic in stripe-synchronizer (line 305) will cause it to exit when customer has a business name instead of when missing
  • Critical: Extra space in cron pattern for rollup-engine (line 217) results in invalid cron syntax

Confidence Score: 2/5

  • This PR has critical logic and syntax errors that will cause runtime failures
  • While the overall migration pattern is sound and consistently applied across 10+ applications, two critical bugs prevent safe merging: (1) inverted boolean logic in stripe-synchronizer will cause incorrect control flow, and (2) malformed cron pattern in rollup-engine will fail to schedule properly. These are production-breaking issues that must be fixed.
  • Pay close attention to stripe-synchronizer/serverlessFunctions/stripe/src/index.ts (inverted logic) and rollup-engine/serverlessFunctions/calculaterollups/src/index.ts (cron syntax)

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-apps/hacktoberfest-2025/rollup-engine/serverlessFunctions/calculaterollups/src/index.ts 2/5 Migrated to twenty-cli:0.2.0 with config export. Critical: cron pattern has extra space causing invalid syntax.
packages/twenty-apps/hacktoberfest-2025/stripe-synchronizer/serverlessFunctions/stripe/src/index.ts 2/5 Migrated to twenty-cli:0.2.0. Critical: inverted logic at line 305 - checks if businessName exists when should check if missing.
packages/twenty-apps/hacktoberfest-2025/activity-summary/serverlessFunctions/summarise-and-send/src/index.ts 5/5 Migrated to twenty-cli:0.2.0 with config export. Code formatting improved, no issues found.
packages/twenty-cli/src/constants/base-application-project/tsconfig.json 5/5 Added allowSyntheticDefaultImports to base tsconfig template for better module compatibility.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant CLI as twenty-cli:0.2.0
    participant App as Hacktoberfest Apps
    participant Manifest as Serverless Manifests
    participant Config as Application Configs

    Dev->>App: Update applications
    App->>Manifest: Remove .manifest.jsonc files
    App->>Config: Add application.config.ts
    Config-->>App: Define app metadata & variables
    App->>App: Export config in index.ts
    Note over App: Add ServerlessFunctionConfig<br/>with triggers defined in code
    App->>CLI: Sync with twenty-cli
    CLI->>Config: Read application.config.ts
    CLI->>App: Parse exported config from index.ts
    CLI-->>Dev: Applications synced successfully
Loading

Additional Comments (1)

  1. packages/twenty-apps/hacktoberfest-2025/rollup-engine/serverlessFunctions/calculaterollups/src/index.ts, line 217 (link)

    syntax: extra space before the cron pattern causes invalid cron syntax

72 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +305 to 308
if (stripeCustomer?.businessName) {
console.warn('Set customer business name in Stripe');
return {};
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: logic is inverted - should check if businessName is missing (falsy), not present

Suggested change
if (stripeCustomer?.businessName) {
console.warn('Set customer business name in Stripe');
return {};
}
if (!stripeCustomer?.businessName) {
console.warn('Set customer business name in Stripe');
return {};
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-apps/hacktoberfest-2025/stripe-synchronizer/serverlessFunctions/stripe/src/index.ts
Line: 305:308

Comment:
**logic:** logic is inverted - should check if `businessName` is missing (falsy), not present

```suggestion
    if (!stripeCustomer?.businessName) {
      console.warn('Set customer business name in Stripe');
      return {};
    }
```

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

Copy link
Copy Markdown
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

LGTM

@Weiko Weiko merged commit 86a6e04 into main Nov 4, 2025
45 checks passed
@Weiko Weiko deleted the fix-hacktoberfest-applications branch November 4, 2025 17:40
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