Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

@icecrasher321 icecrasher321 commented Jan 23, 2026

Summary

  • Adding conflict target prevents insertion errors that end webhook execution.
  • Remove db level namespace tracking -- that's already in the key. Separation can remain at the config level. Only affects the DB fallback not redis version.

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

vercel bot commented Jan 23, 2026

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 23, 2026 2:28am

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 23, 2026

Greptile Summary

This PR simplifies the idempotency system by removing the namespace column from the database schema and embedding namespace information directly in the normalized key instead. The database table is refactored from a composite primary key (key, namespace) to a single-column primary key on key. The corresponding application code is updated to remove namespace field insertions and adjust the conflict resolution targets in database queries to match the new schema. This refactoring reduces schema complexity while maintaining the same idempotency guarantees through the key normalization approach that was already in place.

Confidence Score: 5/5

  • This PR is safe to merge with no concerns. The schema simplification is well-implemented, all code changes are consistent with the new schema, and the refactoring maintains idempotency guarantees.
  • Score of 5 reflects: (1) Complete consistency between database schema changes, migration, and application code - no mismatches detected; (2) Proper use of Drizzle ORM's onConflictDoNothing/onConflictDoUpdate with correct target specification; (3) The refactoring logically simplifies the schema without losing functionality - namespace embedding in keys was already the design pattern; (4) No breaking changes to the API or behavior of the idempotency service; (5) Migration is straightforward with proper cleanup of old indexes; (6) Thorough context from exploration shows this change aligns with recent work on namespace handling.
  • No files require special attention. All changes are consistent and well-coordinated across the codebase.

Important Files Changed

Filename Overview
apps/sim/lib/core/idempotency/service.ts Removed namespace field from insert operations in atomicallyClaimDb() and storeResultDb() functions. Updated onConflictDoNothing() target from [idempotencyKey.key, idempotencyKey.namespace] to idempotencyKey.key to match the new single-column primary key. All database operations now use only the normalized key. Changes are consistent with the schema migration that removed the namespace column.
packages/db/schema.ts Updated idempotencyKey table schema to remove the namespace column and associated indexes. Changed from a composite primary key (key, namespace) to a single-column primary key key. Removed keyNamespacePk unique index and namespaceIdx index. The createdAtIdx index is retained for cleanup operations. This simplification leverages namespace information being embedded in the key itself rather than stored as a separate column.
packages/db/migrations/0147_rare_firebrand.sql Migration that executes the schema changes: drops namespace-related indexes, adds a primary key on key column, and removes the namespace column from the idempotency_key table. Migration is straightforward and aligns with the application code changes.

Sequence Diagram

sequenceDiagram
    participant Client
    participant WebhookExecution as Webhook Execution
    participant IdempotencyService
    participant Database
    
    Client->>WebhookExecution: Send webhook
    WebhookExecution->>IdempotencyService: executeWithIdempotency(provider, id)
    
    IdempotencyService->>IdempotencyService: normalizeKey (namespace:provider:id)
    IdempotencyService->>IdempotencyService: atomicallyClaim()
    
    IdempotencyService->>Database: INSERT with onConflictDoNothing(target: key)
    
    alt First Request (claim succeeds)
        Database-->>IdempotencyService: Insert successful
        IdempotencyService->>WebhookExecution: claimed: true
        WebhookExecution->>WebhookExecution: Execute operation
        WebhookExecution->>IdempotencyService: storeResult()
        IdempotencyService->>Database: INSERT/UPDATE with onConflictDoUpdate(target: key)
        Database-->>IdempotencyService: Result stored
        IdempotencyService-->>WebhookExecution: Success
        WebhookExecution-->>Client: 200 OK
    else Duplicate Request (conflict)
        Database-->>IdempotencyService: Conflict on key
        IdempotencyService->>Database: SELECT existing result
        Database-->>IdempotencyService: Return cached result
        IdempotencyService->>WebhookExecution: claimed: false, existingResult
        WebhookExecution-->>Client: Return cached result
    end
Loading

@icecrasher321
Copy link
Collaborator Author

@greptile

@icecrasher321
Copy link
Collaborator Author

@cursor review

@icecrasher321 icecrasher321 changed the title fix(idempotency): add conflict target to atomicallyClaimDb query fix(idempotency): add conflict target to atomicallyClaimDb query + remove redundant db namespace tracking Jan 23, 2026
cursor[bot]

This comment was marked as outdated.

@icecrasher321
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.

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

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

@icecrasher321 icecrasher321 merged commit 1b309b5 into staging Jan 23, 2026
11 checks passed
@icecrasher321 icecrasher321 deleted the fix/idempotency-check branch January 23, 2026 02:38
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