Skip to content

Conversation

@Sg312
Copy link
Contributor

@Sg312 Sg312 commented Nov 22, 2025

Summary

Brief description of what this PR does and why.

Fixes #(issue)

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

How has this been tested? What should reviewers focus on?

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

@vercel
Copy link

vercel bot commented Nov 22, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Nov 22, 2025 0:59am

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 22, 2025

Greptile Overview

Greptile Summary

Fixes webhook triggers becoming unsaved when using the new diff store by adding server-side webhook persistence during workflow saves and improved 404 handling in the webhook management hook.

Key Changes:

  • Added syncWorkflowWebhooks() function in the workflow state API to persist webhook metadata (webhookId, triggerId, providerConfig, etc.) to the database during workflow saves
  • Refactored useWebhookManagement hook to extract createWebhook() and updateWebhook() functions for better code organization
  • Added 404 error handling in updateWebhook() that automatically recreates missing webhooks by clearing the stored webhookId and calling createWebhook()
  • Added clarifying comment in mergeSubblockState() explaining that orphaned values include runtime webhook identifiers

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk, addressing a specific bug in webhook persistence
  • The implementation is focused and addresses a real bug. The server-side sync function properly handles both update and recreation scenarios. The hook refactoring improves code organization without changing core logic. Minor concerns around edge cases and error handling could be improved but don't block merging.
  • Pay attention to apps/sim/app/api/workflows/[id]/state/route.ts - the new sync function runs after workflow save and could have race conditions with concurrent webhook updates

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/app/api/workflows/[id]/state/route.ts 4/5 Added syncWorkflowWebhooks function to persist webhook metadata during workflow save, preventing webhook data loss in the new diff store
apps/sim/hooks/use-webhook-management.ts 4/5 Refactored webhook saving logic into separate createWebhook and updateWebhook functions, added 404 handling to recreate missing webhooks
apps/sim/stores/workflows/utils.ts 5/5 Added clarifying comment explaining that orphaned subblock values include runtime IDs like webhookId and triggerPath

Sequence Diagram

sequenceDiagram
    participant User
    participant Frontend
    participant SubBlockStore
    participant WorkflowAPI
    participant WebhookAPI
    participant Database

    User->>Frontend: Edit workflow with webhook triggers
    Frontend->>SubBlockStore: Store webhookId, triggerId, triggerConfig
    User->>Frontend: Save workflow
    Frontend->>WorkflowAPI: PUT /api/workflows/[id]/state
    WorkflowAPI->>Database: Save workflow state to normalized tables
    WorkflowAPI->>WorkflowAPI: syncWorkflowWebhooks()
    
    loop For each block with webhookId
        WorkflowAPI->>Database: Check if webhook exists
        alt Webhook exists
            WorkflowAPI->>Database: Update webhook if needed (workflowId, blockId, path)
        else Webhook missing (404)
            WorkflowAPI->>Database: Recreate webhook with stored metadata
            WorkflowAPI->>WorkflowAPI: Log webhook recreation
        end
    end
    
    WorkflowAPI-->>Frontend: Success response
    
    Note over User,Database: Hook's updateWebhook now handles 404 by recreating
    
    User->>Frontend: Update webhook config later
    Frontend->>WebhookAPI: PATCH /api/webhooks/[id]
    alt Webhook found
        WebhookAPI->>Database: Update providerConfig
        WebhookAPI-->>Frontend: Success
    else Webhook not found (404)
        WebhookAPI-->>Frontend: 404 error
        Frontend->>Frontend: useWebhookManagement detects 404
        Frontend->>SubBlockStore: Clear webhookId
        Frontend->>WebhookAPI: POST /api/webhooks (recreate)
        WebhookAPI->>Database: Insert new webhook
        Frontend->>SubBlockStore: Store new webhookId
    end
Loading

Copy link
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.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines 339 to 354
if (needsUpdate) {
await db
.update(webhook)
.set({
workflowId,
blockId: block.id,
path: triggerPath,
provider: provider || existing.provider,
providerConfig: Object.keys(providerConfig).length
? providerConfig
: existing.providerConfig,
isActive: true,
updatedAt: new Date(),
})
.where(eq(webhook.id, webhookId))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: only updates webhook if blockId, workflowId, or path changed, but doesn't update providerConfig or provider if they changed but those fields stayed the same

this means if a user updates their trigger configuration (e.g., changes credentials or trigger settings), those changes won't be persisted to the webhook during workflow save unless the blockId/workflowId/path also changed

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/api/workflows/[id]/state/route.ts
Line: 339:354

Comment:
**logic:** only updates webhook if `blockId`, `workflowId`, or `path` changed, but doesn't update `providerConfig` or `provider` if they changed but those fields stayed the same

this means if a user updates their trigger configuration (e.g., changes credentials or trigger settings), those changes won't be persisted to the webhook during workflow save unless the blockId/workflowId/path also changed

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

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