Skip to content

fix(import): dedup workflow name#3813

Merged
icecrasher321 merged 1 commit intostagingfrom
fix/import-dedup
Mar 27, 2026
Merged

fix(import): dedup workflow name#3813
icecrasher321 merged 1 commit intostagingfrom
fix/import-dedup

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

Summary

Correctly dedup workflow name on user import

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

vercel bot commented Mar 27, 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 27, 2026 8:05pm

Request Review

@cursor
Copy link
Copy Markdown

cursor bot commented Mar 27, 2026

PR Summary

Low Risk
Low risk: change is opt-in via a new deduplicate flag and only affects workflow creation naming/duplicate handling during imports and template seeding.

Overview
Adds an optional deduplicate flag to POST /api/workflows to automatically pick a unique name (via deduplicateWorkflowName) instead of returning a 409 on name collisions.

Updates workflow creation callers used by imports/landing seeds (use-import-workflow, use-import-workspace, and home.tsx) plus the useCreateWorkflow mutation to pass deduplicate: true, so imported workflows won’t fail when a name already exists in the target folder.

Written by Cursor Bugbot for commit cbab38b. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 27, 2026

Greptile Summary

This PR fixes workflow import by switching from a hard 409-conflict error to an automatic name-deduplication strategy. When deduplicate: true is passed to POST /api/workflows, the deduplicateWorkflowName utility is called to append (2), (3), … to the requested name rather than rejecting the request. The flag is enabled for all three import paths (single-workflow import, workspace import, and landing-template creation in home.tsx), and the shared CreateWorkflowVariables type is updated to carry it through.\n\nKey points:\n- The deduplication utility (deduplicateWorkflowName) was pre-existing; this PR simply wires it into the API route behind the new flag.\n- The telemetry call was correctly moved to after the dedup/conflict-check block, so it no longer fires for requests that would have been rejected.\n- Existing callers that don't pass deduplicate retain the previous 409 behavior unchanged.\n- A theoretical race condition exists between the name-uniqueness check and the subsequent INSERT — two concurrent imports could both receive the same candidate name and both succeed, leaving duplicate names in the DB. In practice this is unlikely for typical single-user sequential imports, but worth noting for future hardening.

Confidence Score: 5/5

Safe to merge — the change is focused, the dedup utility is pre-existing and tested, and all remaining findings are P2.

No P0 or P1 issues found. The only concern (race condition between the uniqueness check and the insert) is P2 because it requires simultaneous concurrent imports to the same workspace+folder to manifest, and the worst-case outcome is two workflows with the same name rather than data loss or a crash.

apps/sim/app/api/workflows/route.ts — the dedup check + insert gap is worth revisiting if concurrent imports become a supported scenario.

Important Files Changed

Filename Overview
apps/sim/app/api/workflows/route.ts Core API change: adds deduplicate flag that switches from 409-error behavior to auto-rename via deduplicateWorkflowName; telemetry call correctly moved after the dedup/duplicate-check block
apps/sim/lib/workflows/utils.ts Pre-existing deduplicateWorkflowName utility; issues N+1 DB queries in a loop (up to 98 round-trips) — not introduced by this PR but worth awareness for bulk imports
apps/sim/app/workspace/[workspaceId]/w/hooks/use-import-workflow.ts Single-line addition of deduplicate: true to createWorkflowMutation.mutateAsync call — straightforward and correct
apps/sim/app/workspace/[workspaceId]/w/hooks/use-import-workspace.ts Single-line addition of deduplicate: true to the workspace-import workflow creation body — correct; since it targets a brand-new workspace there will rarely be conflicts, so dedup adds only one extra DB query per workflow
apps/sim/app/workspace/[workspaceId]/home/home.tsx Adds deduplicate: true when creating a workflow from a landing template seed — prevents 409 errors when users click a template more than once
apps/sim/hooks/queries/workflows.ts Adds deduplicate?: boolean to CreateWorkflowVariables interface and passes it through to the POST body — clean, type-safe plumbing

Sequence Diagram

sequenceDiagram
    participant Client
    participant API as POST /api/workflows
    participant Dedup as deduplicateWorkflowName
    participant DB as Database

    Client->>API: POST { name, deduplicate: true }
    API->>Dedup: deduplicateWorkflowName(name, workspaceId, folderId)
    Dedup->>DB: SELECT id WHERE name = 'name' (check original)
    DB-->>Dedup: exists / not exists
    alt name is taken
        loop i = 2..99
            Dedup->>DB: SELECT id WHERE name = 'name (i)'
            DB-->>Dedup: exists / not exists
        end
    end
    Dedup-->>API: unique candidate name
    Note over API,DB: race window — another request could claim same name here
    API->>DB: INSERT workflow with candidate name
    DB-->>API: new workflow row
    API->>Client: { id, name, ... }
Loading

Reviews (1): Last reviewed commit: "fix(import): dedup workflow name" | Re-trigger Greptile

Comment on lines 203 to 210
sortOrder = minSortOrder != null ? minSortOrder - 1 : 0
}

const duplicateConditions = [
eq(workflow.workspaceId, workspaceId),
isNull(workflow.archivedAt),
eq(workflow.name, name),
]
let name = requestedName

if (folderId) {
duplicateConditions.push(eq(workflow.folderId, folderId))
if (deduplicate) {
name = await deduplicateWorkflowName(requestedName, workspaceId, folderId)
} else {
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.

P2 Race condition between dedup check and insert

deduplicateWorkflowName computes a unique name with a DB read, but the actual insert happens several lines later with no transaction or lock. If two concurrent import requests check the same workspace+folder+name at the same time, both will receive the same "available" candidate (e.g. My Workflow (2)) and both will successfully insert it, resulting in duplicate names in the database.

For single-user sequential imports this is practically harmless, but it is theoretically possible when two browser tabs or API calls run simultaneously. Wrapping the dedup check and the db.insert(...) in a database transaction, or alternatively using an upsert/ON CONFLICT strategy, would eliminate the window.

This is worth keeping in mind, especially for the bulk workspace-import path where many workflows are created in a tight loop.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

not an actual problem, db would correctly not do it

@icecrasher321 icecrasher321 merged commit 2dd6d3d into staging Mar 27, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/import-dedup branch March 27, 2026 22:33
waleedlatif1 pushed a commit that referenced this pull request Mar 28, 2026
waleedlatif1 added a commit that referenced this pull request Mar 28, 2026
…rm (#3824)

* fix(import): dedup workflow name (#3813)

* feat(concurrency): bullmq based concurrency control system (#3605)

* feat(concurrency): bullmq based queueing system

* fix bun lock

* remove manual execs off queues

* address comments

* fix legacy team limits

* cleanup enterprise typing code

* inline child triggers

* fix status check

* address more comments

* optimize reconciler scan

* remove dead code

* add to landing page

* Add load testing framework

* update bullmq

* fix

* fix headless path

---------

Co-authored-by: Theodore Li <teddy@zenobiapay.com>

* fix(linear): add default null for after cursor (#3814)

* fix(knowledge): reject non-alphanumeric file extensions from document names (#3816)

* fix(knowledge): reject non-alphanumeric file extensions from document names

* fix(knowledge): improve error message when extension is non-alphanumeric

* fix(security): SSRF, access control, and info disclosure (#3815)

* fix(security): scope copilot feedback GET endpoint to authenticated user

Add WHERE clause to filter feedback records by the authenticated user's
ID, preventing any authenticated user from reading all users' copilot
interactions, queries, and workflow YAML (IDOR / CWE-639).

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

* fix(smtp): add SSRF validation and genericize network error messages

Prevent SSRF via user-controlled smtpHost by validating with
validateDatabaseHost before creating the nodemailer transporter.
Collapse distinct network error messages (ECONNREFUSED, ECONNRESET,
ETIMEDOUT) into a single generic message to prevent port-state leakage.

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

* fix(security): add SSRF validation to SFTP/SSH and access control to workspace invitations

Add `validateDatabaseHost` checks to SFTP and SSH connection utilities to
block connections to private/reserved IPs and localhost, matching the
existing pattern used by all database tools. Add authorization check to
the workspace invitation GET endpoint so only the invitee or a workspace
admin can view invitation details.

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

* fix(smtp): restore SMTP response code handling for post-connection errors

SMTP 4xx/5xx response codes are application-level errors (invalid
recipient, mailbox full, server error) unrelated to the SSRF hardening
goal. Restore response code differentiation and logging to preserve
actionable user-facing error messages.

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

* fix(security): use session email directly instead of extra DB query

Addresses PR review feedback — align with the workspace invitation
route pattern by using session.user.email instead of re-fetching
from the database.

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

* lint

* fix(auth): revert lint autofix that broke hasExternalApiCredentials return type

Biome auto-fixed `return auth !== null && auth.startsWith(...)` to
`return auth?.startsWith(...)` which returns `boolean | undefined`,
not `boolean`, causing a TypeScript build failure.

* fix(smtp): pin resolved IP to prevent DNS rebinding (TOCTOU)

Use the pre-resolved IP from validateDatabaseHost instead of the
original hostname when creating the nodemailer transporter. Set
servername to the original hostname to preserve TLS SNI validation.

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

* refactor(security): extract createPinnedLookup helper for DNS rebinding prevention

Extract reusable createPinnedLookup from secureFetchWithPinnedIP so
non-HTTP transports (SSH, SFTP, IMAP) can pin resolved IPs at the
socket level. SMTP route uses host+servername pinning instead since
nodemailer doesn't reliably pass lookup to both secure/plaintext paths.

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

* fix(security): pin IMAP connections to validated resolved IP

Pass the resolved IP from validateDatabaseHost to ImapFlow as host,
with the original hostname as servername for TLS SNI verification.
Closes the DNS TOCTOU rebinding window.

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

* lint

* fix(auth): revert lint autofix on hasExternalApiCredentials return type

Also pin SFTP/SSH connections to validated resolved IP to prevent DNS rebinding.

* fix(security): short-circuit admin check when caller is invitee

Skip the hasWorkspaceAdminAccess DB query when the caller is already
the invitee, avoiding an unnecessary round-trip. Aligns with the org
invitation route pattern.

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

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* fix(worker): dockerfile + helm updates (#3818)

* fix(worker): dockerfile + helm updates

* address comments

* update dockerfile (#3819)

* fix dockerfile

* fix(security): pentest remediation — condition escaping, SSRF hardening, ReDoS protection (#3820)

* fix(executor): escape newline characters in condition expression strings

Unescaped newline/carriage-return characters in resolved string values
cause unterminated string literals in generated JS, crashing condition
evaluation with a SyntaxError.

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

* fix(security): prevent ReDoS in guardrails regex validation

Add safe-regex2 to reject catastrophic backtracking patterns before
execution and cap input length at 10k characters.

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

* fix(security): SSRF localhost hardening and regex DoS protection

Block localhost/loopback URLs in hosted environments using isHosted flag
instead of allowHttp. Add safe-regex2 validation and input length limits
to regex guardrails to prevent catastrophic backtracking.

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

* fix(security): validate regex syntax before safety check

Move new RegExp() before safe() so invalid patterns get a proper syntax
error instead of a misleading "catastrophic backtracking" message.

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

* fix(security): address PR review feedback

- Hoist isLocalhost && isHosted guard to single early-return before
  protocol checks, removing redundant duplicate block
- Move regex syntax validation (new RegExp) before safe-regex2 check
  so invalid patterns get proper syntax error instead of misleading
  "catastrophic backtracking" message

* fix(security): remove input length cap from regex validation

The 10k character cap would block legitimate guardrail checks on long
LLM outputs. Input length doesn't affect ReDoS risk — the safe-regex2
pattern check already prevents catastrophic backtracking.

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

* fix(tests): mock isHosted in input-validation and function-execute tests

Tests that assert self-hosted localhost behavior need isHosted=false,
which is not guaranteed in CI where NEXT_PUBLIC_APP_URL is set to the
hosted domain.

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

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* improvement(worker): configuration defaults (#3821)

* improvement(worker): configuration defaults

* update readmes

* realtime curl import

* improvement(tour): remove auto-start, only trigger on explicit user action (#3823)

* fix(mcp): use correct modal for creating workflow MCP servers in deploy (#3822)

* fix(mcp): use correct modal for creating workflow MCP servers in deploy

* fix(mcp): show workflows field during loading and when empty

* mock course

* fix(db): use bigint for token counter columns in user_stats (#3755)

* mock course

* updates

* updated X handle for emir

* cleanup: audit and clean academy implementation

* fix(academy): add label to ValidationRule, fix quiz gating, simplify getRuleMessage

* cleanup: remove unnecessary comments across academy files

* refactor(academy): simplify abstractions and fix perf issues

* perf(academy): convert course detail page to server component with client island

* fix(academy): null-safe canAdvance, render exercise instructions, remove stale comments

* fix(academy): remove orphaned migration, fix getCourseById, clean up comments

- Delete 0181_academy_certificate.sql (orphaned duplicate not in journal)
- Add getCourseById() to content/index.ts; use it in certificates API
  (was using getCourse which searches by slug, not stable id)
- Remove JSX comments from catalog page
- Remove redundant `passed` recomputation in LessonQuiz

* chore(db): regenerate academy_certificate migration with drizzle-kit

* chore: include blog mdx and components changes

* fix(blog): correct cn import path

* fix(academy): constrain progress bar to max-w-3xl with proper padding

* feat(academy): show back-to-course button on first lesson

* fix(academy): force dark theme on all /academy routes

* content(academy): rewrite sim-foundations course with full 6-module curriculum

* fix(academy): correct edge handles, quiz explanation, and starter mock outputs

- Fix Exercise 2 initial edge handles: 'starter-1-source'/'agent-1-target' → 'source'/'target' (React Flow actual IDs)
- Fix M1-L4 Q4 quiz explanation: remove non-existent Ctrl/Cmd+D and Alt+drag shortcuts
- Add starter mock output to all exercises so run animation shows feedback on the first block

* refine(academy): fix inaccurate content and improve exercise clarity

- Fix Exercise 3: replace hardcoded <agent-1.content> (invalid UUID-based ref) with reference picker instructions
- Fix M4 Quiz Q5: Loop block (subflow container) is correct answer, not the Workflow block
- Fix M4 Quiz Q4: clarify fan-out vs Parallel block distinction in explanation
- Fix M4-L2 video description: accurately describe Loop and Parallel subflow blocks
- Fix M2 Quiz Q3: make response format question conceptual rather than syntax-specific
- Improve Exercise 4 branching instructions: clarify top=true / bottom=false output handles
- Improve Final Project instructions: step-by-step numbered flow

* fix(academy): remove double border on quiz question cards

* fix(academy): single scroll container on lesson pages — remove nested flex scroll

* fix(academy): remove min-h-screen from root layout — fixes double scrollbar on lesson pages

* fix(academy): use fixed inset-0 on lesson page to eliminate document-level scrollbar

* fix(academy): replace sr-only radio/checkbox inputs with buttons to prevent scroll-on-focus; restore layout min-h-screen

* improvement(academy): polish, security hardening, and certificate claim UI

- Replace raw localStorage with BrowserStorage utility in local-progress
- Pre-compute slug/id Maps in content/index for O(1) course lookups
- Move blockMap construction into edge_exists branch only in validation
- Extract navBtnClass constant and MetaRow/formatDate helpers in UI
- Add rate limiting, server-side completion verification, audit logging, and nanoid cert numbers to certificate issuance endpoint
- Add useIssueCertificate mutation hook with completedLessonIds
- Wire certificate claim UI into CourseProgress: sign-in prompt, claim button with loading state, and post-issuance view with link to certificate page
- Fix lesson page scroll container and quiz scroll-on-focus bug

* fix(academy): validate condition branch handles in edge_exists rules

- Add sourceHandle field to edge_exists ValidationRule type
- Check sourceHandle in validation.ts when specified
- Require both condition-if and condition-else branches to be connected in the branching and final project exercises

* fix(academy): address PR review — isHosted regression, stuck isExecuting, revoked cert 500, certificate SSR

- Restore env-var-based isHosted check (was hardcoded true, breaking self-hosted deployments)
- Fix isExecuting stuck at true when mock run fails validation — set isMockRunningRef immediately and reset both flags on early exit
- Fix revoked/expired certificate causing 500 — any existing record (not just active) now returns 409 instead of falling through to INSERT
- Convert certificate verification page from client component to server component — direct DB fetch, notFound() on missing cert, generateMetadata for SEO/social previews

* fix(auth): restore hybrid.ts from staging to fix CI type error

* fix(academy): mark video lessons complete on visit and fix sign-in path

* fix(academy): replace useEffect+setState with lazy useState initializer in CourseProgress

* fix(academy): reset exerciseComplete on lesson navigation, remove unused useAcademyCertificate hook

* fix(academy): useState for slug-change reset, cache() for cert page, handleMockRunRef for stale closure

* fix(academy): replace shadcn theme vars with explicit hex in LessonVideo fallback

* fix(academy): reset completedRef on exercise change, conditional verified badge, multi-select empty guard

* fix(academy): type safety fixes — null metadata fallbacks, returning() guard, exhaustive union, empty catch

* fix(academy): reset ExerciseView completed banner on nav; fix CourseProgress hydration mismatch

* fix(lightbox): guard effect body with isOpen to prevent spurious overflow reset

* fix(academy): reset LessonQuiz state on lesson change to prevent stale answers persisting

* fix(academy): course not-found metadata title; try-finally guard in mock run loop

* fix(academy): type safety, cert persistence, regex guard, mixed-lesson video, shorts support

- Derive AcademyCertificate from db $inferSelect to prevent schema drift
- Add useCourseCertificate query hook; GET /api/academy/certificates now accepts courseId for authenticated lookup
- Use useCourseCertificate in CourseProgress so certificate state survives page refresh
- Guard new RegExp(valuePattern) in validation.ts with try/catch; log warn on invalid pattern
- Add logger.warn for custom validation rules so content authors are alerted
- Add YouTube Shorts URL support to LessonVideo (youtube.com/shorts/VIDEO_ID)
- Fix mixed-lesson video gap: render videoUrl above quiz when mixed has quiz but no exercise
- Add academy-scoped not-found.tsx with link back to /academy

* fix(academy): reset hintIndex when exercise changes

* chore: remove ban-spam-accounts script (wrong branch)

* fix(academy): enforce availableBlocks in toolbar; fix mixed exercise+quiz rendering

- Add useSandboxBlockConstraints context; SandboxCanvasProvider provides exerciseConfig.availableBlocks so the toolbar only shows permitted block types. Empty array hides all blocks (configure-only exercises); non-null array restricts to listed types; triggers always hidden in sandbox.
- Fix mixed lesson with both exerciseConfig and quizConfig: exercise renders first, quiz reveals after exercise completes (sequential pedagogy). canAdvance now requires both exerciseComplete && quizComplete when both are present.

* chore(academy): remove extraneous inline comments

* fix(academy): blank mixed lesson, quiz canAdvance flag, empty-array valueNotEmpty

* prep for merge

* chore(db): regenerate academy certificate migration after staging merge

* fix(academy): disable auto-connect in sandbox mode

* fix(academy): render video in mixed lesson with no exercise or quiz

* fix(academy): mark mixed video-only lessons complete; handle cert insert race

* fix(canvas): add sandbox and embedded to nodes useMemo deps

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Lakee Sivaraya <71339072+lakeesiv@users.noreply.github.com>
Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>
Co-authored-by: Vikhyath Mondreti <vikhyathvikku@gmail.com>
Co-authored-by: Siddharth Ganesan <33737564+Sg312@users.noreply.github.com>
Co-authored-by: Theodore Li <teddy@zenobiapay.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