Skip to content

feat(runtime): add idempotency key support to trigger()#6710

Merged
Hundao merged 1 commit into
aden-hive:mainfrom
Rohit23SR:feat/webhook-idempotency-5947
Apr 1, 2026
Merged

feat(runtime): add idempotency key support to trigger()#6710
Hundao merged 1 commit into
aden-hive:mainfrom
Rohit23SR:feat/webhook-idempotency-5947

Conversation

@Rohit23SR

Copy link
Copy Markdown
Contributor

Fixes #5947

Adds an optional idempotency_key param to trigger() and trigger_and_wait(). If the same key comes in again within the TTL window (default 5 min), we return the cached execution_id instead of starting a new run. Cache uses OrderedDict with TTL expiry and FIFO eviction at 10k keys. Placeholders + async lock handle the race between concurrent retries.

Backward compatible — defaults to None, existing callers unchanged.

20 tests added.

@Hundao

Hundao commented Mar 22, 2026

Copy link
Copy Markdown
Collaborator

There's a race condition with the placeholder. If trigger A is mid-execution and trigger B hits with the same key, B gets back "pending-abc123", which isn't a real execution_id. Caller can't do anything useful with that.

How about dropping the placeholder entirely? Just don't write to the cache until execute() finishes. That way the cache only ever holds real execution_ids. Webhook retries are usually seconds to minutes apart, so two triggers hitting within milliseconds shouldn't really happen in practice.

Also, this uses self._lock which is the same lock as start/stop. Separate _idempotency_lock would be cleaner.

@Rohit23SR
Rohit23SR force-pushed the feat/webhook-idempotency-5947 branch from fe439b6 to 7067d48 Compare March 22, 2026 21:09
@Rohit23SR

Copy link
Copy Markdown
Contributor Author

Good catch — you're right, returning a pending placeholder breaks the contract since callers use the ID for tracking.

I dropped the placeholder and the lock entirely. The cache only gets written after execute() returns, so it always holds a real execution_id. No separate lock needed either — without a placeholder there's nothing to protect atomically, and the dict operations between awaits run on the event loop without interleaving.

The tradeoff is a tiny window where a concurrent trigger could slip through, but execute() is non-blocking (just creates a task and returns), so that window is microseconds — and webhook retries are seconds apart anyway.

Pushed the update.

@Rohit23SR

Copy link
Copy Markdown
Contributor Author

Hey @Hundao — just checking in. I pushed the update dropping the placeholder and lock as discussed. Let me know if there's anything else you'd like changed.

@Hundao
Hundao merged commit 9774e53 into aden-hive:main Apr 1, 2026
7 checks passed
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.

Add idempotency key support for webhook-triggered executions to prevent duplicates

2 participants