Skip to content

Enforce name uniqueness#3679

Open
TheodoreSpeaks wants to merge 8 commits intostagingfrom
fix/enforce-duplicate-name
Open

Enforce name uniqueness#3679
TheodoreSpeaks wants to merge 8 commits intostagingfrom
fix/enforce-duplicate-name

Conversation

@TheodoreSpeaks
Copy link
Collaborator

@TheodoreSpeaks TheodoreSpeaks commented Mar 19, 2026

Summary

Going forward, resources should have unique names. Added validation and error throwing on duplicate names and also added the unique name constraint to knowledge bases.

Resources with duplicate names would trigger a toast popup saying that the name is not unique.

Type of Change

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

Testing

Validated migration, validated that creating resources works correctly and throws error.

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

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Mar 20, 2026 2:12am

Request Review

@TheodoreSpeaks TheodoreSpeaks marked this pull request as ready for review March 19, 2026 19:55
@cursor
Copy link

cursor bot commented Mar 19, 2026

PR Summary

Medium Risk
Adds database-level uniqueness for active knowledge base names and introduces conflict-aware create/rename/restore flows that can change restored entity names and return 409s. Moderate risk due to new unique index/migration and altered restore/rename behavior under concurrency.

Overview
Enforces workspace-scoped name uniqueness for active knowledge bases (new migration dedupes existing duplicates and adds a partial unique index on (workspace_id, name) where not deleted).

Updates knowledge base and table services to detect/translate Postgres unique violations into new *ConflictError types, and adjusts API routes to return HTTP 409 on those conflicts (including restore endpoints). Restore flows for knowledge bases, tables, and workspace files now generate a unique restored name (e.g. _restored + hash) and retry on concurrent unique violations.

Frontend behavior is tweaked to surface these failures: React Query mutations no longer retry by default, key mutations show error toasts, and the toast/notification countdown ring is extracted/reused with toast auto-dismiss pausing on hover.

Written by Cursor Bugbot for commit 5ca1d5b. This will update automatically on new commits. Configure here.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 19, 2026

Greptile Summary

This PR enforces name uniqueness for knowledge bases, tables, and workspace files — both preventing duplicate creation/renaming and gracefully resolving conflicts when restoring archived entities by auto-generating a unique _restored suffix. It also extracts the countdown-ring SVG into a shared EMCN component and adds pause-on-hover support to the toast system.

Key changes:

  • KnowledgeBaseConflictError and TableConflictError typed error classes with 409 API responses
  • Application-level SELECT guards in createKnowledgeBase / updateKnowledgeBase for duplicate name detection (no DB constraint backs these)
  • renameTable relies on catching the DB unique-constraint violation (23505) — more race-condition-safe but uses a fragile Drizzle error-cause shape
  • generateRestoreName utility: tries original → _restored_restored_{hex} without verifying uniqueness of the final hash-based candidate
  • CountdownRing extracted into apps/sim/components/emcn/components/toast/countdown-ring.tsx but not exported from the emcn barrel, causing both toast.tsx and notifications.tsx to use subpath imports in violation of the EMCN component rule
  • retry: false on mutations prevents retrying idempotency-violating 409 errors
  • onError toast handlers added to useUpdateKnowledgeBase, useRenameTable, and useRenameWorkspaceFile

Confidence Score: 3/5

  • Mostly safe to merge with minor TOCTOU and import-convention issues that should be addressed
  • The core conflict-detection and restore-name logic is sound, and the end-to-end plumbing (service → API → mutation → toast) is correctly wired. However, the knowledge base uniqueness check has a TOCTOU race condition with no DB-level constraint to back it up, the CountdownRing component violates the EMCN barrel-export rule, and the Drizzle error-cause shape used to detect 23505 in renameTable is fragile across dependency versions.
  • Pay attention to apps/sim/lib/knowledge/service.ts (TOCTOU race in duplicate name checks) and apps/sim/components/emcn/components/toast/countdown-ring.tsx (missing barrel export causing subpath import violations in two files)

Important Files Changed

Filename Overview
apps/sim/lib/core/utils/restore-name.ts New utility that generates unique names for restored entities using a three-step fallback (original → _restored → restored{hex}); the third step is returned without a uniqueness check.
apps/sim/lib/knowledge/service.ts Adds name-uniqueness enforcement for create/update/restore operations using application-level SELECT guards; susceptible to TOCTOU race conditions because no DB unique constraint backs the check.
apps/sim/lib/table/service.ts Rename now catches the DB-level 23505 unique-constraint violation and maps it to TableConflictError; restore uses generateRestoreName. Error-cause shape assumption is fragile across Drizzle versions.
apps/sim/components/emcn/components/toast/countdown-ring.tsx New reusable SVG countdown ring extracted from notifications.tsx; violates the EMCN rule of not importing from subpaths since it isn't re-exported from the emcn barrel.
apps/sim/components/emcn/components/toast/toast.tsx Adds pause-on-hover logic with correct remaining-time tracking; imports CountdownRing via subpath instead of a relative import within the same directory.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/notifications/notifications.tsx Extracts CountdownRing into a shared component; imports it via subpath violating the EMCN barrel rule. Renames local wrapper to NotificationCountdownRing to avoid naming conflicts.
apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts Restore now generates a unique name via generateRestoreName (with hasExtension:true) before unsetting deletedAt; logic is straightforward and correctly updates originalName in the schema.
apps/sim/hooks/queries/kb/knowledge.ts Adds onError toast for useUpdateKnowledgeBase so 409 conflict responses are surfaced to users; straightforward addition.
apps/sim/hooks/queries/tables.ts Adds onError toast for useRenameTable; correctly extracts the error message from the API response before throwing.
apps/sim/hooks/queries/workspace-files.ts Adds onError toast for useRenameWorkspaceFile; consistent with the pattern added to the other query hooks in this PR.
apps/sim/app/api/knowledge/route.ts Handles KnowledgeBaseConflictError by returning 409 before the generic 500 handler; straightforward and correct.
apps/sim/app/api/table/[tableId]/route.ts Handles TableConflictError with a 409 in the PATCH handler; correct placement in the existing error-handling chain.
apps/sim/app/_shell/providers/get-query-client.ts Disables mutation retries (retry: false) so that 409 conflict errors are immediately surfaced to onError rather than being retried, which would be incorrect for idempotency violations.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User renames / creates entity] --> B{Entity type?}

    B -->|Knowledge Base| C[SELECT duplicate check\nby workspaceId + name]
    C -->|Duplicate found| D[throw KnowledgeBaseConflictError]
    C -->|No duplicate| E[INSERT / UPDATE KB]
    D --> F[API returns 409]
    F --> G[useUpdateKnowledgeBase\nonError → toast.error]

    B -->|Table| H[UPDATE userTableDefinitions\ncatch DB error 23505]
    H -->|Caught 23505| I[throw TableConflictError]
    H -->|Success| J[Return updated table]
    I --> K[API returns 409]
    K --> L[useRenameTable\nonError → toast.error]

    B -->|Workspace File| M[PATCH /files/:id\ncatch non-ok response]
    M -->|Error| N[useRenameWorkspaceFile\nonError → toast.error]

    subgraph Restore Flow
        O[Restore archived entity] --> P[generateRestoreName]
        P --> Q{originalName free?}
        Q -->|Yes| R[Use original name]
        Q -->|No| S{name_restored free?}
        S -->|Yes| T[Use name_restored]
        S -->|No| U[Use name_restored_hex\n⚠ not verified unique]
        R & T & U --> V[UPDATE set deletedAt=null\n+ new name]
    end
Loading

Last reviewed commit: "Fix lint"

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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

)
)

logger.info(`Successfully restored workspace file: ${fileRecord.name}`)
Copy link

Choose a reason for hiding this comment

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

Workspace file restore lacks race-condition protection unlike peers

Low Severity

restoreWorkspaceFile calls generateRestoreName and then performs the DB update non-atomically, without a transaction or retry loop. The analogous restoreTable and restoreKnowledgeBase functions both use FOR UPDATE locks inside transactions with retry loops for 23505 errors. While workspace files lack a unique index on names (so no DB error occurs), a concurrent upload or restore can claim the chosen name between the check and the update, silently producing duplicate display names.

Fix in Cursor Fix in Web

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