Skip to content

Idempotency keys only dedupe task.trigger() calls — the docs list "avoiding double-charging customers" as a use case, but a payment call made directly inside run() gets no protection #4627

Description

@aurumflux20

Hi — really like how deliberately this project has thought about retries (per-task config, exponential backoff, staging/prod on by default with dev opt-out documented plainly). This is about a gap in the idempotency system itself, from reading docs/idempotency.mdx and packages/trigger-sdk/src/v3/idempotencyKeys.ts against 69f396f. No runtime testing — happy to be corrected if this is handled somewhere I didn't find.

The shape

docs/idempotency.mdx lists this as a use case (line 35):

Avoiding double-charging customers - Prevent duplicate payment processing during retries

But every code example in the same doc — including the one directly under that bullet — only shows idempotency protecting childTask.trigger():

const idempotencyKey = await idempotencyKeys.create("my-task-key");
await childTask.trigger({ foo: "bar" }, { idempotencyKey });

And idempotencyKeys.ts confirms this is the entire surface:

export const idempotencyKeys = {
  create: createIdempotencyKey,
  reset: resetIdempotencyKey,
};

There is no primitive that makes an arbitrary side effect idempotent — only task.trigger()/triggerAndWait() calls consume an idempotency key. A payment call made directly inside run(), rather than moved into a child task, gets no protection from this system at all.

Why that's a real gap, not just a docs nit

Retries are on by default outside dev (docs/errors-retrying.mdx: "By default when you create your project using the CLI init command we disabled retrying in the DEV environment" — implying prod/staging retry by default), and the docs' own canonical retry example is:

export const myTask = task({
  id: "my-task",
  retry: { maxAttempts: 4 },
  run: async (payload) => {
    const idempotencyKey = await idempotencyKeys.create("my-task-key");
    await childTask.trigger({ foo: "bar" }, { idempotencyKey });
    throw new Error("Something went wrong");   // triggers a retry of THIS task
  },
});

A developer who read "avoiding double-charging customers" as a supported use case, and naturally reached for the pattern shown immediately below it, would write:

run: async (payload) => {
  await stripe.refunds.create({ payment_intent: payload.piId, amount: payload.amount });
  await sendConfirmationEmail(payload);   // throws for an unrelated reason
}

The email step failing retries the whole task (per the configured maxAttempts), and the refund call — having no idempotency key of its own — runs again on the retry. The task-level idempotency key never touches it, because nothing in this task calls trigger().

Suggested fix

Two independent options, not mutually exclusive:

  1. Narrow the doc claim precisely: state that idempotency keys dedupe task triggers, not arbitrary code inside run(), and that the correct pattern for "avoid double-charging" is to always wrap the payment call in a child task triggered with a key — never call a payment API directly inside a retryable run().
  2. Or close the actual gap: a general-purpose helper — something like idempotencyKeys.run(key, fn) — that persists the result of an arbitrary async operation keyed the same way trigger() already is, so a direct API call inside run() can opt into the same protection without being restructured into a child task.

Either would stop the doc's own payments use case from silently not applying to the code shown right next to it.

Activity

  1. bharathkumar39293 commented on Aug 17, 2026

    @bharathkumar39293
    Contributor

    Thanks for raising this — I verified the current implementation, and the core observation still applies: Trigger.dev idempotency keys deduplicate task-trigger requests, not arbitrary side effects executed inside run().

    I opened #4650 to clarify that boundary in the documentation and add a Stripe refund example.

    One important nuance: moving a payment into an idempotently triggered child task prevents duplicate child runs caused by retries of the parent, but it still does not by itself guarantee exactly-once execution of the payment API call. The child task can retry or crash after the provider accepts a request.

    For payments, the robust pattern is:

    Trigger.dev idempotency to deduplicate task triggers;
    provider-side idempotency (for example, Stripe’s idempotency key) for the external request; and
    durable application state where additional business-operation coordination is needed.

    A generic idempotencyKeys.run(key, fn) helper could be useful in some form, but it cannot safely promise universal exactly-once semantics for arbitrary external side effects: there is always a crash window between the provider accepting a request and durable completion being recorded.

    The docs PR therefore focuses on making the existing guarantee precise rather than overstating what a general-purpose wrapper could guarantee.

  2. leadingproblemsolver commented on Aug 25, 2026

    @leadingproblemsolver

    One additional boundary seems worth naming explicitly: deduplication is not the same as settlement after ambiguous execution.

    Provider-side idempotency makes retrying the same request safe while that provider key is valid. But after a timeout/crash where the provider may already have accepted the mutation, the runtime still has an uncertain state unless it can establish what happened externally.

    A pattern I've been testing is:

    1. persist ACTION_PENDING before dispatching the side effect;
    2. dispatch once;
    3. if the worker dies / response is ambiguous, a resumed worker must reconcile the external system before any retry;
    4. if the effect is present, settle it without replay; only if reconciliation proves it absent is retry permitted.

    So the transition is closer to ACTION_PENDING -> reconcile -> VERIFIED | retryable, rather than exception -> retry.

    This matters for providers without idempotency keys, for queryable side effects, and also after provider idempotency retention windows expire. For non-queryable effects, I don't think there is a universal runtime guarantee; you still need a provider/business-operation primitive such as idempotency, an outbox, or equivalent coordination.

    Is this intentionally an application-owned responsibility in Trigger.dev, or is there a runtime hook/state where a task can represent "side effect may have happened; reconcile before retry"? If it is application-owned, that distinction may be useful to state alongside the provider-idempotency guidance.

  3. aurumflux20 commented on Aug 28, 2026

    @aurumflux20
    Author

    @leadingproblemsolver that distinction is the right one, and I think it's the part the docs PR can't cover — because it isn't a documentation gap, it's a missing runtime state.

    Dedup answers "is this the same request?" Settlement answers "did the effect happen?" Provider idempotency gives you the first. Nothing gives you the second once the worker dies mid-call, because the answer doesn't live in your process — it lives at the provider.

    The state machine you sketched is exactly what I ended up building, with one addition: the ambiguous state has to be terminal until reconciled, never retryable-by-default. Concretely:

    • persist the claim before dispatch (a durable row, not in-process state — two replicas share no memory);
    • dispatch once;
    • on an ambiguous failure, keep the claim and do not release it. Releasing a claim is what lets the next attempt charge again, so the failure path must refuse to guess;
    • reconcile against the provider on demand: exactly one effect found → settle it without replay; authoritatively absent → release for a clean retry; more than one → that's a divergence, freeze the path; couldn't reach an answer → change nothing, loudly.

    That last branch is the one that matters. A timeout, a 5xx and a rate-limit all mean "unknown", and collapsing "unknown" into "didn't happen" is the double-charge in one line of code.

    I implemented this as an open-source kernel on Postgres if it's useful as a reference — settle(intent, witness) is that transition: https://github.com/aurumflux20/seal

    For Trigger.dev specifically, my read is it stays application-owned (the provider is the only party who can answer), but a first-class side_effect_pending → reconcile state would let the runtime stop pretending a failed activity means an un-run one. Worth stating alongside the provider-idempotency guidance either way.

  4. leadingproblemsolver commented on Aug 29, 2026

    @leadingproblemsolver

    I traced this one level deeper against the current run engine and against aurumflux20/seal, and I think the missing boundary is now concrete.

    On the worker side, packages/core/src/v3/workers/taskExecutor.ts turns a thrown task error into a failed TaskRunExecutionResult, with retry populated when #handleError() chooses retry. That result flows into RunAttemptSystem.attemptFailed().

    The decisive point is then internal-packages/run-engine/src/engine/retrying.ts::retryOutcomeFromCompletion(). Its outcome type is currently only:

    • cancel_run
    • fail_run
    • retry

    For any retriable error with attempts remaining, it returns retry; runAttemptSystem.ts::attemptFailed() then records the retry outcome and either requeues the run or creates a new EXECUTING snapshot for an immediate retry.

    So the runtime currently has no durable outcome equivalent to "the task failed, but an external side effect may already have happened; automatic retry is unsafe until reconciled." That is the exact place where exception -> retry collapses an epistemically-unknown external state into a retryable one.

    I also traced the seal implementation you linked. Its useful invariant is narrower than a generic idempotency helper:

    • Gateway.execute() catches an ordinary post-dispatch exception as AmbiguousOutcome and deliberately does not release the durable claim.
    • Seal.settle() later asks a registered witness/provider:
      • exactly one effect -> heal/finalize without replay
      • authoritatively absent -> release for a clean retry
      • multiple -> mark divergence + freeze
      • unknown -> preserve the claim and change nothing

    That suggests a minimal Trigger.dev runtime question rather than a whole feature proposal:

    should RetryOutcome have a fourth durable outcome such as pending_reconciliation, which causes attemptFailed() to persist a non-runnable snapshot/state instead of calling recordRetryOutcome() / requeueing?

    A minimal failing scenario would be:

    1. retryable task calls a queryable external provider;
    2. provider accepts the mutation;
    3. worker loses the response and throws;
    4. current engine schedules another attempt;
    5. desired behavior under an explicit reconciliation contract: run remains blocked/non-runnable until a witness says present / absent / divergent / still unknown.

    I have not proved yet where provider-specific reconciliation belongs in Trigger.dev, so I would not jump to a PR from this alone. But I think the runtime insertion point is now localized to the TaskRunExecutionResult -> retryOutcomeFromCompletion() -> attemptFailed() transition, and the missing semantic is a durable non-retryable unknown-effect state rather than another idempotency key.

  5. aurumflux20 commented on Aug 31, 2026

    @aurumflux20
    Author

    This is a precise read of seal, and your localization of the insertion point is right — the missing element is a durable outcome, not another key. pending_reconciliation as a fourth RetryOutcome that persists a non-runnable snapshot instead of requeueing is the same move seal makes one layer down: Gateway.execute() holds the claim on an ambiguous post-dispatch failure precisely so the settle step can't schedule a blind retry.

    One thing from having built it: the fourth state only pays off if the reconcile read has somewhere to run. In seal that's the witness registry — a per-provider "did this land?" returning present / absent / multiple / unknown. Without a registered witness, pending_reconciliation is a durable block with no exit — safe, but the run stalls. So the runtime hook and the provider primitive are two halves: the engine needs the non-runnable state (your retryOutcomeFromCompletion → attemptFailed point), and the application needs to register how to answer it (your step 3). Neither alone closes it.

    So "is this application-owned?" is really "does the runtime own a witness interface, or just the state?" A defensible split is: the engine owns the durable unknown-effect state and the block; the application registers the reconciler per side effect. That keeps the engine from having to know about Stripe or any specific provider while still refusing the unsafe automatic retry — which is the exact seam the MCP retry-timing SEP (#3188) is circling from the client side.

  6. Srinivasan8888 commented on Aug 31, 2026

    @Srinivasan8888

    Working on the docs side of this — the "avoiding double-charging customers" bullet needs to say plainly that idempotency keys scope to trigger() calls and do not protect side effects performed directly inside run().

    Vouch request open at #4845 — holding the PR until vouched.

  7. postdvk commented on Aug 31, 2026

    @postdvk

    @bharathkumar39293

    The proposed run/journal primitive solves deterministic replay inside Trigger, but I’m curious about the boundary where fn reaches an external provider (Stripe/webhook/API), the provider commits, and the response is lost before Trigger records success.

    Would a durable idempotency key plus provider-native lookup be considered sufficient to resolve that state, or do you expect provider-specific reconciliation logic per integration?

    If a generic read-only layer could return applied / not_applied / unknown bound to the idempotency key, request digest and freshness, would that remove useful work or just move provider-specific logic elsewhere?

  8. Srinivasan8888 commented on Aug 31, 2026

    @Srinivasan8888

    Withdrawing my comment above — I missed #4650 (@bharathkumar39293), which already covers the docs half of this on the same file, and I'd read only the first part of this thread when I commented. It was closed the day it was opened by the vouch gate, not on its merits.

    Having now read the rest: @leadingproblemsolver and @aurumflux20's localization of the runtime gap to retryOutcomeFromCompletion() → attemptFailed() is a different and larger question than the docs wording, and I don't have anything to add to it. Deferring to #4650 for the documentation side.

  9. aurumflux20 commented on Sep 1, 2026

    @aurumflux20
    Author

    @postdvk That question is the seam exactly, and having built the read-only layer you're describing, my answer is: it moves the provider-specific logic rather than removing it — but the split it creates is the whole value, so the move is worth making.

    What generalizes is the verdict space and the rule for reading it. What stays provider-specific is only the read itself: which endpoint answers "did this land?" for a given call. That is a much smaller surface than per-integration reconciliation logic, and critically it's a surface the application already owns — it knows it called Stripe, so it can register how to ask Stripe. The runtime never has to learn a provider. That's the same split @bharathkumar39293's docs land on from the other side: engine owns the durable state, application owns the answer.

    One correction from having shipped it, and it's the reason I'd push back on the exact shape you proposed: applied / not_applied / unknown is one state short. In practice you need four:

    • applied once — settle it, don't replay.
    • authoritatively not applied — replay is now provably safe.
    • applied more than once — a prior retry already double-fired. This is a divergence to surface, not a state to retry from, and folding it into "applied" hides the incident you most need to see. This state is how you find out your retry policy has been quietly double-charging.
    • could not determine — the reconciliation read itself failed, timed out, or the provider can't answer.

    Your instinct to bind the answer to freshness is right, and it's precisely what makes that fourth state load-bearing: a stale or failed read must not collapse into not_applied. If unknown degrades to "didn't happen," you have reintroduced the exact double-fire the layer existed to prevent — now with a green check on it. So unknown has to be terminal for automatic handling: stop and surface, never replay.

    On @bharathkumar39293's point that no wrapper can promise universal exactly-once because of the crash window between provider-accept and durable-record — that's correct and I don't think it's worth trying to argue around. The window can't be closed from inside the runtime. But "can't be prevented" and "can't be resolved" are different claims, and the second one is false. You can't stop the ambiguity from occurring; you can make it answerable afterward instead of guessed at. That reframing is what makes a durable non-runnable state useful rather than just a stall: the run parks in "unknown effect," and the registered read is the exit.

    This is written up as normative text in the retry-safety proposal I've been co-authoring with @YoadElkayam — the four-verdict space and the "could not determine is terminal, never absent" rule are §4.3: https://github.com/YoadElkayam/mcp-fuse/tree/main/sep. Fair warning on status: it is a working draft with no sponsor yet, not an adopted standard. Framed for MCP clients rather than task runners, but the seam is the same one this issue is about, and the conformance suite in that repo catches the failure directly — it caught two implementations replaying a side-effecting call off a received response, on the reasoning that a 503 or a reset message means the effect didn't land. It can arrive after the effect landed.

  10. matt-aitken commented on Sep 18, 2026

    @matt-aitken
    Member

    This is a good note on making the docs clearer. I've updated that in #4952 and #4953.

    When using a 3rd party system or provider, like Stripe, the only way to get true idempotency is to send them an idempotency key. The docs now reflect this.

    They also make clear that the idempotency is at the task level.

    We won't be adding an idempotency primitive inside tasks at the moment, users can use tasks for this as it provides the same level of protection.

  11. aurumflux20 commented on Sep 18, 2026

    @aurumflux20
    Author

    Thank you — and I think you landed it in the right place.

    Two things worth saying plainly, since this thread will outlive the fix.

    You are right about the mechanism, and the docs are the correct fix. "The only way to get true idempotency is to send them an idempotency key" is the honest statement, and it is stronger than a runtime primitive would have been, because a primitive would have made the platform look responsible for a guarantee only the provider can actually give. Declining to add one is the more defensible call, not the lazier one.

    On "users can use tasks for this, as it provides the same level of protection" — that holds precisely when the retry boundary and the idempotency boundary are the same boundary. Moving the payment into its own task makes them the same. It is worth being explicit about that in the docs if it is not already, because the failure I filed happens exactly when someone believes those two boundaries coincide and they do not.

    Filed 15 Aug, docs shipped 18 Sep in #4952 and #4953. I will record it that way on the public index — found and fixed, with attribution to you, and no company is ever named while a finding is open.

    Nothing owed here either way. Closing the loop because a thread that ends in a fix should say so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions