Skip to content

fix(chat): use explicit trigger type check instead of heuristic for chat guard#3419

Merged
waleedlatif1 merged 3 commits intostagingfrom
waleedlatif1/richmond-v2
Mar 5, 2026
Merged

fix(chat): use explicit trigger type check instead of heuristic for chat guard#3419
waleedlatif1 merged 3 commits intostagingfrom
waleedlatif1/richmond-v2

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • Replace !isExecutingFromChat heuristic with explicit overrideTriggerType !== 'chat' check in execution completion callbacks
  • The isExecutingFromChat heuristic ('input' in workflowInput) could false-positive for manual executions with a user-defined "input" test field, leaving isExecuting stuck true forever
  • All 3 call sites of executeWorkflow always pass an explicit trigger type ('chat', 'manual'), so this check is deterministic

Type of Change

  • Bug fix

Testing

Verified all 3 executeWorkflow call sites always pass explicit overrideTriggerType. Type check passes clean.

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 Mar 5, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 5, 2026 3:01am

Request Review

@cursor
Copy link

cursor bot commented Mar 5, 2026

PR Summary

Low Risk
Low risk single-condition change in use-workflow-execution that affects when execution cleanup is skipped for chat runs; only risky if any caller fails to pass overrideTriggerType correctly.

Overview
Switches the isExecutingFromChat guard in use-workflow-execution.ts to rely solely on overrideTriggerType === 'chat', removing the heuristic that treated any workflowInput containing an input field as chat.

This prevents manual/test executions that happen to include an input key from being misclassified as chat and skipping the normal completion cleanup (e.g., leaving isExecuting stuck true).

Written by Cursor Bugbot for commit fec4793. Configure here.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 5, 2026

Greptile Summary

This PR fixes a false-positive bug in the isExecutingFromChat guard inside executeWorkflow. The old implementation included a heuristic ('input' in workflowInput) that could match manual test executions with a user-defined input field, causing isExecuting to remain stuck as true after the run completed. The fix correctly replaces the compound condition with the deterministic overrideTriggerType === 'chat' check.

Key observations:

  • All three internal call sites of executeWorkflow already supply an explicit overrideTriggerType ('chat' at line 942, 'manual' at line 1055, and 'manual' inline at line 2111), confirming the heuristic fallback was always dead code in practice.
  • The overrideTriggerType parameter remains optional in the function signature. If a future caller omits it, isExecutingFromChat defaults to false, which is the safe outcome — state cleanup will run as normal rather than being skipped.
  • The fix correctly preserves all downstream uses of isExecutingFromChat: streaming forwarding (line 1393), execution-completed cleanup guard (line 1499), error-path cleanup guard (line 1542), and cancellation-path cleanup guard (line 1568).

Confidence Score: 5/5

  • This PR is safe to merge — it is a minimal, targeted bug fix with no risk of regression.
  • The change is a single-line simplification that removes a provably incorrect heuristic. All three call sites pass an explicit trigger type, the logic is straightforward, and the default-to-false behavior for any future caller without an explicit type is the safer outcome. No new code paths are introduced.
  • No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts Removes the heuristic 'input' in workflowInput fallback from the isExecutingFromChat guard, replacing it with the explicit overrideTriggerType === 'chat' check. All three call sites already pass an explicit trigger type, making the heuristic both redundant and a source of false positives.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[executeWorkflow called] --> B{overrideTriggerType?}
    B -- "'chat'" --> C[isExecutingFromChat = true]
    B -- "'manual' / 'api' / undefined" --> D[isExecutingFromChat = false]

    C --> E[Fetch selectedOutputs from chat store]
    C --> F[Find chat trigger start block]
    C --> G{onStream && isExecutingFromChat?}
    G -- yes --> H[Forward stream chunks to chat UI]
    G -- no --> I[Skip stream forwarding]

    D --> J[Resolve manual/API start block]

    C --> K[onExecutionCompleted]
    D --> K
    K -- isExecutingFromChat=true --> L[Skip setIsExecuting false\nchat finally-block handles cleanup]
    K -- isExecutingFromChat=false --> M[setIsExecuting false\nclearActiveBlocks]

    C --> N[onExecutionError / onExecutionCancelled]
    D --> N
    N -- isExecutingFromChat=true --> O[Skip state reset\nchat handles cleanup]
    N -- isExecutingFromChat=false --> P[setIsExecuting false\nsetIsDebugging false\nclearActiveBlocks]
Loading

Last reviewed commit: fec4793

Use only overrideTriggerType === 'chat' instead of also checking
for 'input' in workflowInput, which can false-positive on manual
executions with workflow input.
@waleedlatif1
Copy link
Collaborator Author

@greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

Replace inline overrideTriggerType !== 'chat' checks with
!isExecutingFromChat to stay consistent with the rest of the function.
@waleedlatif1
Copy link
Collaborator Author

@greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

@waleedlatif1 waleedlatif1 merged commit 8579beb into staging Mar 5, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/richmond-v2 branch March 5, 2026 03:05
waleedlatif1 added a commit that referenced this pull request Mar 5, 2026
…hat guard (#3419)

* fix(chat): use explicit trigger type check instead of heuristic for chat guard

* fix(chat): remove heuristic fallback from isExecutingFromChat

Use only overrideTriggerType === 'chat' instead of also checking
for 'input' in workflowInput, which can false-positive on manual
executions with workflow input.

* fix(chat): use isExecutingFromChat variable consistently in callbacks

Replace inline overrideTriggerType !== 'chat' checks with
!isExecutingFromChat to stay consistent with the rest of the function.
waleedlatif1 added a commit that referenced this pull request Mar 5, 2026
* feat(slack): add new tools and user selectors

* fix(slack): fix download fileName param and canvas error handling

* fix(slack): use markdown format for canvas rename title_content

* fix(slack): rename channel output to channelInfo and document presence API limitation

* lint

* fix(chat): use explicit trigger type check instead of heuristic for chat guard (#3419)

* fix(chat): use explicit trigger type check instead of heuristic for chat guard

* fix(chat): remove heuristic fallback from isExecutingFromChat

Use only overrideTriggerType === 'chat' instead of also checking
for 'input' in workflowInput, which can false-positive on manual
executions with workflow input.

* fix(chat): use isExecutingFromChat variable consistently in callbacks

Replace inline overrideTriggerType !== 'chat' checks with
!isExecutingFromChat to stay consistent with the rest of the function.

* fix(slack): add missing fields to SlackChannel interface

* fix(slack): fix canvas transformResponse type mismatch

Provide required output fields on error path to match SlackCanvasResponse type.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(slack): move error field to top level in canvas transformResponse

The error field belongs on ToolResponse, not inside the output object.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.

1 participant