Skip to content

fix(execution): duplicate execution issue - #6316

Merged
icecrasher321 merged 1 commit into
stagingfrom
fix/dup-stream-check
Aug 6, 2026
Merged

fix(execution): duplicate execution issue#6316
icecrasher321 merged 1 commit into
stagingfrom
fix/dup-stream-check

Conversation

@icecrasher321

Copy link
Copy Markdown
Collaborator

Summary

Can now have duplicate executions across tabs/shared sessions and it wont error on replay paths either. Moving server authoritative id gen for idempotency key to prevent races.

Type of Change

  • Bug fix

Testing

Tested manually

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

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 6, 2026 7:39am

Request Review

@cursor

cursor Bot commented Aug 6, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes Copilot workflow claim rejection and client completion/error paths; misclassification could hide real failures or leave duplicate tabs in a bad state, but scope is narrow and covered by tests.

Overview
When a Copilot workflow tool is already bound to another execution, the execute API now returns 409 with a stable COPILOT_WORKFLOW_EXECUTION_CONFLICT code (shared constant) instead of only a generic error string.

The client path treats that response as a duplicate runner, not a failed run: executeWorkflowWithFullLogging throws ExecutionStreamHttpError with status and code and skips terminal HTTP error rows; run-tool-execution logs and drops the duplicate without /api/copilot/confirm error reporting or user-facing failure. ExecutionStreamHttpError now carries an optional code for this classification.

Tests cover the API JSON shape, client classification without console noise, and silent duplicate handling in the run-tool client.

Reviewed by Cursor Bugbot for commit f93338e. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR makes Copilot workflow execution claims server-authoritative so concurrent tabs or shared sessions can safely race without surfacing the losing request as an execution error.

  • Adds a machine-readable conflict code to duplicate-claim 409 responses.
  • Propagates typed HTTP error codes through the workflow execution client.
  • Treats the coded duplicate conflict as benign while cleaning up the losing client’s local execution state.
  • Adds focused route, execution utility, and client-runner tests.

Confidence Score: 5/5

The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issues identified.

The server atomically selects one execution owner, the winner remains responsible for confirming the single shared tool-call record, and the rejected client cleans up only its own tab-local execution state.

Important Files Changed

Filename Overview
apps/sim/app/api/workflows/[id]/execute/route.ts Adds a stable conflict code to the existing atomic Copilot workflow claim rejection while retaining claim cleanup behavior.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.ts Converts only the coded duplicate-claim response into a typed HTTP error and preserves normal console reporting for other failures.
apps/sim/hooks/use-execution-stream.ts Extends the typed execution HTTP error with an optional machine-readable code without breaking existing constructors.
apps/sim/lib/copilot/tools/client/run-tool-execution.ts Suppresses the benign losing side of a duplicate claim race while retaining the existing finally-based local cleanup.
apps/sim/lib/copilot/constants.ts Defines the shared conflict-code constant used consistently by the route and client.
apps/sim/app/api/workflows/[id]/execute/route.async.test.ts Verifies the coded 409 response and that execution does not proceed after a failed claim.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-execution-utils.test.ts Verifies duplicate conflicts are typed and do not create misleading HTTP error console entries.
apps/sim/lib/copilot/tools/client/run-tool-execution.test.ts Verifies a duplicate runner neither confirms nor surfaces an error and still clears its local execution pointer.

Sequence Diagram

sequenceDiagram
  participant A as Client tab A
  participant B as Client tab B
  participant API as Workflow execute API
  participant DB as Copilot tool-call record
  A->>API: Execute with toolCallId
  B->>API: Execute with same toolCallId
  API->>DB: Atomically claim for A
  DB-->>API: Claim acquired
  API-->>A: Start execution stream
  API->>DB: Attempt claim for B
  DB-->>API: Already claimed
  API-->>B: 409 + conflict code
  B->>B: Ignore duplicate and clear local state
  A->>DB: Confirm shared tool-call completion
Loading

Reviews (1): Last reviewed commit: "fix(execution): duplicate execution issu..." | Re-trigger Greptile

@icecrasher321
icecrasher321 merged commit 293dc5b into staging Aug 6, 2026
30 checks passed
@icecrasher321
icecrasher321 deleted the fix/dup-stream-check branch August 6, 2026 07:47
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