Skip to content

Conversation

@Sg312
Copy link
Contributor

@Sg312 Sg312 commented Nov 11, 2025

Summary

Prevents adding triggers to subflows

Type of Change

  • Bug fix

Testing

Manual

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)

@vercel
Copy link

vercel bot commented Nov 11, 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 11, 2025 4:42am

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 11, 2025

Greptile Overview

Greptile Summary

Prevents triggers from being added to loop or parallel subflows through multiple validation layers.

Key Changes:

  • Added TRIGGER_IN_SUBFLOW warning type with user-friendly messaging
  • Implemented three prevention checkpoints:
    1. Blocks dropped directly into containers (workflow.tsx:869-884)
    2. Existing blocks dragged into containers (workflow.tsx:1751-1771)
    3. Trigger mode toggled on blocks already in subflows (store.ts:1054-1066, use-collaborative-workflow.ts:974-989)
  • Added isBlockInSubflow utility method (currently unused)
  • Used custom window event to communicate from collaborative hook to UI

Notes:

  • The unused isBlockInSubflow utility could be leveraged to reduce code duplication across the three validation points

Confidence Score: 5/5

  • Safe to merge - comprehensive validation prevents all trigger-in-subflow scenarios
  • The implementation covers all three code paths where triggers could enter subflows (drop, drag, toggle), uses defensive checks throughout, and follows established codebase patterns. The only minor issue is an unused utility function that could improve code organization.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/lib/workflows/triggers.ts 4/5 Added isBlockInSubflow utility method to check if block is inside loop or parallel subflow
apps/sim/hooks/use-collaborative-workflow.ts 5/5 Added validation in collaborativeToggleBlockTriggerMode to prevent enabling trigger mode inside subflows
apps/sim/stores/workflows/workflow/store.ts 5/5 Added validation in toggleBlockTriggerMode to prevent enabling trigger mode inside loop or parallel subflows
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx 5/5 Added three trigger prevention checks: event listener for warning dialog, validation on drop into container, and validation on drag into container

Sequence Diagram

sequenceDiagram
    participant User
    participant Toolbar/Drop
    participant Workflow.tsx
    participant Store/Hook
    participant TriggerUtils
    participant Dialog

    Note over User,Dialog: Scenario 1: Drop trigger into container
    User->>Workflow.tsx: Drop block into container
    Workflow.tsx->>Workflow.tsx: Check if containerInfo exists
    Workflow.tsx->>Workflow.tsx: Check if block is trigger<br/>(category='triggers' OR triggers.enabled OR enableTriggerMode)
    alt Is trigger block
        Workflow.tsx->>TriggerUtils: getDefaultTriggerName(type)
        TriggerUtils-->>Workflow.tsx: Return trigger name
        Workflow.tsx->>Dialog: Show TRIGGER_IN_SUBFLOW warning
        Dialog-->>User: Display "Triggers not allowed in subflows"
        Note over Workflow.tsx: Early return - block not added
    else Not trigger block
        Workflow.tsx->>Workflow.tsx: Add block to container
    end

    Note over User,Dialog: Scenario 2: Drag existing trigger into container
    User->>Workflow.tsx: Drag block over container
    Workflow.tsx->>Workflow.tsx: onNodeDragStop with potentialParentId
    Workflow.tsx->>Workflow.tsx: Get block from blocks[node.id]
    Workflow.tsx->>TriggerUtils: isTriggerBlock(block)
    TriggerUtils-->>Workflow.tsx: true/false
    alt Is trigger block
        Workflow.tsx->>TriggerUtils: getDefaultTriggerName(block.type)
        TriggerUtils-->>Workflow.tsx: Return trigger name
        Workflow.tsx->>Dialog: Show TRIGGER_IN_SUBFLOW warning
        Dialog-->>User: Display "Triggers not allowed in subflows"
        Workflow.tsx->>Workflow.tsx: Reset drag state (no parent update)
    else Not trigger block
        Workflow.tsx->>Workflow.tsx: Update parent relationship
    end

    Note over User,Dialog: Scenario 3: Toggle trigger mode on block in subflow
    User->>Store/Hook: Toggle trigger mode
    Store/Hook->>Store/Hook: Check if newTriggerMode is true
    Store/Hook->>Store/Hook: Check if block.data.parentId exists
    Store/Hook->>Store/Hook: Get parent block
    alt Parent is loop or parallel
        Store/Hook->>Store/Hook: Log warning (store only)
        alt In collaborative hook
            Store/Hook->>Workflow.tsx: Dispatch 'show-trigger-warning' event
            Workflow.tsx->>Dialog: Show TRIGGER_IN_SUBFLOW warning
            Dialog-->>User: Display "Triggers not allowed in subflows"
        end
        Note over Store/Hook: Early return - mode not changed
    else Parent not loop/parallel or no parent
        Store/Hook->>Store/Hook: Toggle trigger mode
        Store/Hook->>Store/Hook: Remove incoming edges if enabling
    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.

5 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +574 to +594
static isBlockInSubflow<T extends { id: string; data?: { parentId?: string } }>(
blockId: string,
blocks: T[] | Record<string, T>
): boolean {
const blockArray = Array.isArray(blocks) ? blocks : Object.values(blocks)
const block = blockArray.find((b) => b.id === blockId)

if (!block || !block.data?.parentId) {
return false
}

// Check if the parent is a loop or parallel block
const parent = blockArray.find((b) => b.id === block.data?.parentId)
if (!parent) {
return false
}

// Type-safe check: parent must have a 'type' property
const parentWithType = parent as T & { type?: string }
return parentWithType.type === 'loop' || parentWithType.type === 'parallel'
}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Unused utility - isBlockInSubflow is never called. The same logic is duplicated inline in use-collaborative-workflow.ts:977 and store.ts:1057. Consider either using this utility or removing it to reduce code duplication.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/lib/workflows/triggers.ts
Line: 574:594

Comment:
**style:** Unused utility - `isBlockInSubflow` is never called. The same logic is duplicated inline in `use-collaborative-workflow.ts:977` and `store.ts:1057`. Consider either using this utility or removing it to reduce code duplication.

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

@Sg312 Sg312 merged commit 991b0e3 into staging Nov 11, 2025
9 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/triggers-in-subflows branch November 11, 2025 18:25
@waleedlatif1 waleedlatif1 mentioned this pull request Nov 12, 2025
10 tasks
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